Skip to content

fix: restrict the local API to the diff under review and same-origin callers - #40

Open
jessebond2 wants to merge 2 commits into
wong2:mainfrom
jessebond2:harden-local-server
Open

fix: restrict the local API to the diff under review and same-origin callers#40
jessebond2 wants to merge 2 commits into
wong2:mainfrom
jessebond2:harden-local-server

Conversation

@jessebond2

Copy link
Copy Markdown

The local server answers any caller that can reach the port, and /api/file-content serves any file inside the repository. Two commits, one concern each.

Symlink containment

  • isSafePath compares lexically, so a symlink stored inside the repo served the contents of whatever it pointed at outside the repo.
  • Adds resolveWithinDir (realpath-based) for the worktree read. The git show path is untouched — it returns a link's target text rather than following it.

API scope

  • /api/file-content now serves only paths in the diff under review. Previously any repository file was readable, including gitignored ones such as .env.
  • Membership comes from git diff --name-only -z rather than parsing the patch: the diff --git header is ambiguous for paths containing b/ (as the /api/file-versions comment already notes), and binary files carry no +++ header to read instead. Side effect: images whose path contains a space now display, where they previously 404'd.
  • /api/* requires a loopback Host, and a same-origin Origin when one is present. This stops a page the user happens to have open from posting comments (which the agent skills then apply as code changes) or writing settings, and stops a rebound DNS name from reading the diff.
  • PUT /api/settings validates keys and values instead of spreading the request body, since browser names the application launched on the next run. It is limited to the values the Toolbar offers.

Kept working on purpose

  • Requests with no Origin still pass, so the CLI and the diffx-* skills are unaffected — only browsers attach that header.
  • --host 0.0.0.0 still serves LAN reviewers. The Host check is skipped there, since a non-loopback bind is an explicit opt-in and LAN addresses can't be allowlisted ahead of time; the Origin check still applies.
  • getUntrackedFilePaths gained -z. Without it, git quotes paths containing non-ASCII or newline characters, and the new membership check would have regressed those files.
  • softWrap is now declared in the server's Settings. The client already sends it and it was persisting only via the unvalidated spread, so validation would otherwise have dropped it.

Containment was checked lexically, so a symlink stored inside the
repository served the contents of whatever it pointed at outside the
repository.
…lers
/api/file-content served any file inside the repository, so a gitignored
.env was readable by anything that could reach the port. It now serves
only paths in the diff being reviewed, asked of git by name rather than
parsed back out of the patch: the `diff --git` header is ambiguous for
paths containing ` b/`, and binary files carry no `+++` header to read
instead.
The API also answered any caller: a page the user has open could post
comments (which the agent skills then apply as code changes) or write
settings, and a rebound DNS name could read the diff. Requests are now
checked for a loopback Host and a same-origin Origin. A missing Origin
still passes so the CLI and agent skills keep working, and binding to a
non-loopback host stays an explicit opt-in to LAN access.
Settings writes are validated against known keys and values, since
`browser` names the application launched on the next run.
Sign up for freeto 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.

1 participant

@jessebond2