Skip to content

fix(server): reject foreign directory hints before instance lookup - #40857

Open
romanilyin wants to merge 5 commits into
anomalyco:devfrom
romanilyin:directory-boundary
Open

fix(server): reject foreign directory hints before instance lookup#40857
romanilyin wants to merge 5 commits into
anomalyco:devfrom
romanilyin:directory-boundary

Conversation

@romanilyin

Copy link
Copy Markdown

Supersedes #34256, which was closed by automated PR cleanup.

Issue for this PR

Closes #34255

Part of #33107

Related: #30895, #19473, #36902

Type of change

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

What does this PR do?

Validates request directory hints before instance and location lookup.

  • Query values are consumed once and encoded SDK headers are decoded once.
  • The v1 workspace routing and v2 LocationMiddleware use the same ServerDirectory parser.
  • POSIX/WSL servers reject Windows drive and UNC roots, including slash-form UNC hints.
  • Windows servers reject POSIX-rooted hints instead of silently converting them.
  • Native and relative paths remain compatible, including parent-relative Windows hints such as ../repo.
  • Invalid hints return a structured 400 with the correct query/header source and field.

The existing @romanilyin/canonicalpath dependency remains intentionally unchanged in this revision. This PR does not translate Windows paths to WSL paths, migrate persisted SQLite/App state, change session matching, or resolve symlink identity.

How did you verify your code works?

  • bun install --frozen-lockfile passed with Bun 1.3.14.
  • Focused server suite passed: 57 tests across server-directory, v1 instance/workspace routing, v2 location routing, and SDK coverage.
  • bun typecheck passed in both packages/server and packages/opencode.
  • Workspace-wide bun typecheck passed: 30/30 Turbo tasks.
  • bun run generate in packages/client produced no diff.
  • git diff --check origin/dev..HEAD passed.

The full packages/opencode suite completed with 3259 passed, 22 skipped, 1 todo, and 10 failures. The failures are confined to unrelated subprocess timeout cases in test/cli/acp/lifecycle.test.ts and test/cli/run/run-process.test.ts; both groups reproduce when run in isolation in this WSL workspace. The changed server tests remain green.

Screenshots / recordings

N/A - server/API change.

Checklist

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

@romanilyin

Copy link
Copy Markdown
Author

Updated replacement branch is now based on current dev (def7220bf) and ready for review.

Changes since the closed PR head:

  • Rebased the complete directory-boundary series with no content conflicts.
  • Adapted the shared parser and both current routing paths (WorkspaceRoutingMiddleware and v2 LocationMiddleware).
  • Kept @romanilyin/canonicalpath@2026.6.19-1 as requested; no in-tree replacement was introduced.
  • Preserved relative Windows directory hints, including ../repo and ..\\repo, while still rejecting drive-relative, URI, foreign POSIX-rooted, Windows drive, and UNC syntax where appropriate.
  • Added profile-independent regression coverage for the relative-path compatibility case.

Verification:

  • Focused server suite: 57 passed, 0 failed.
  • bun typecheck in packages/server: passed.
  • bun typecheck in packages/opencode: passed.
  • Workspace bun typecheck: 30/30 Turbo tasks passed.
  • bun run generate in packages/client: no generated diff.
  • git diff --check origin/dev..HEAD: passed.
  • Pre-push workspace typecheck: 30/30 Turbo tasks passed.

Full-suite caveat: packages/opencode completed with 3259 passed, 22 skipped, 1 todo, and 10 failures. All failures are unrelated subprocess/lifecycle timeouts in test/cli/acp/lifecycle.test.ts and test/cli/run/run-process.test.ts, and both groups reproduce in isolation in this WSL workspace. The changed server tests remain green.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: POSIX server resolves foreign Windows directory hints against its cwd

1 participant