Skip to content

fix(server): reject foreign directory hints before location lookup - #54619

Closed
romanilyin wants to merge 2 commits into
anomalyco:v2from
romanilyin:canonicalpath-v2
Closed

fix(server): reject foreign directory hints before location lookup#54619
romanilyin wants to merge 2 commits into
anomalyco:v2from
romanilyin:canonicalpath-v2

Conversation

@romanilyin

@romanilyin romanilyin commented Oct 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Issue for this PR

Closes #54617. Part of #54615; ports the narrow fix from #40857 to v2.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Validate query/header directory hints before all four request-based Location lookups: middleware, global forms, debug eviction and PTY connect. Foreign rooted syntax returns typed InvalidRequestError (400) before lookup.

The server-local wrapper uses @romanilyin/[email protected]. Existing relative/default paths, query precedence and single transport decoding are preserved. Database and client-state identity remain separate umbrella stages.

How did you verify your code works?

  • 33 server tests / 243 assertions passed, including HTTP/SDK, foreign path profiles, fetch and PTY regressions.
  • Server/Protocol/Client typechecks and root bun run check passed on WSL with Bun 1.4.2.
  • Client generation produced no diff.
  • Root check used the target contents of two tracked custom-elements.d.ts symlinks because this Windows checkout stores them as plain text; their original bytes were restored before commit.

Native Windows server integration has not yet been exercised. Maintainer feedback is requested on the dependency/wrapper choice; I can adapt the server-local wrapper to an in-tree implementation if preferred.

Screenshots / recordings

Not applicable; this changes HTTP validation.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@romanilyin
fix(server): validate V2 directory boundaries
64936cc 
Port the request-boundary fix from anomalyco#40857 at 6192c2b to the current
V2 Location middleware, global forms, debug eviction and PTY transport.
Keep the existing single-decode, relative-path and default behavior.
@romanilyin
chore(server): update canonicalpath to 2026.10.10-4
654e225 
Pin the latest published release and refresh its lockfile integrity.
Keep the existing server-boundary API and behavior.
@github-actions

github-actions Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@romanilyin romanilyin mentioned this pull request Oct 11, 2026
Closed
1 of 6 tasks
@opencode-agent opencode-agent Bot mentioned this pull request Oct 11, 2026
Open
@opencode-agent opencode-agent Bot mentioned this pull request Oct 26, 2026
Open
3 of 6 tasks
@romanilyin
romanilyin marked this pull request as ready for review October 26, 2026 21:51
@romanilyin

romanilyin commented Oct 26, 2026

Copy link
Copy Markdown
Author

I have marked this PR ready for review. It covers the request-boundary fix in #54617: reject foreign rooted directory hints before Location lookup across middleware, global forms, debug eviction and PTY connect. Persistent path identity and client migrations remain in #54615.

The existing validation is 33 passing server tests / 243 assertions and passing typechecks/root bun run check on WSL. Native Windows server integration has not yet been exercised; that limitation remains in the description.

Could a maintainer review the boundary behavior and the server-local wrapper? The implementation uses @romanilyin/[email protected]; I can adapt it to an in-tree implementation if that is preferred.

@opencode-agent opencode-agent Bot closed this Oct 26, 2026
@opencode-agent

opencode-agent Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for putting this together, @romanilyin!

The v2 branch was promoted to dev on 2026-10-10, and dev is now where all v2 work happens. This pull request targets v2, which is no longer used, so it is being closed automatically.

Please open it again with dev as the base branch. You can reuse the same head branch; just create a new pull request against dev.

@romanilyin romanilyin mentioned this pull request Oct 26, 2026
Open
3 of 6 tasks
@romanilyin

romanilyin commented Oct 26, 2026

Copy link
Copy Markdown
Author

Replaced by #59414, now open and ready for review against dev. I ported the fix onto b2e4c77621, resolved the conflicts and added validation for the newer MCP client endpoint. Fresh targeted tests, both server typechecks and the root check passed; details are in the replacement PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@romanilyin