Harden the local bridge: pin Host, write tabs.json 0600 - #24
Merged
Conversation
Two gaps in the local bridge's stated security model, both found while reviewing #22. `tabs.json` was written at the umask default (0644) by the shared `atomicWrite`, whose other caller is `init` — where 0644 is right. But this file is the URL and title of every open tab: browsing history, which unlike a token can't be revoked once leaked, and whose full paths carry more than hostnames (account pages, doc links, share links with tokens in the query string). Verified the config dir is *not* reliably 0700 either, so the file mode was the only thing left, and it wasn't set. `atomicWrite` now takes an optional mode, applied to the temp file at creation so the content is never briefly world-readable; the rename makes that inode the target, so no follow-up chmod is needed. `init`'s files are unchanged. The Origin check did not stop DNS rebinding. A page on http://evil.com rebound to 127.0.0.1 keeps its own origin, so its requests are same-origin — and the Fetch spec omits `Origin` on same-origin GET/HEAD, so `GET /script` sailed through and the page could read the response (no CORS between same origins), stealing the queued script and, via pop semantics, denying it to the extension. Confirmed by hand: `curl -H 'Host: evil.example' .../script` returned 200 with the script. Now Host must be 127.0.0.1|localhost:<port>; page JS can't forge it (forbidden header name) and real extension traffic is unaffected. The Origin check stays — browsers always attach Origin to non-GET requests, so it's what blocks a drive-by POST /tabs from writing to disk. The two cover different halves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two gaps in the security model
tabs servedocuments. Both found reviewing #22; split out of #23 so the restructure stayed reviewable.1.
tabs.jsonwas world-readableSame directory, different permissions.
tabs.jsonis written through the sharedatomicWrite, whose other caller isinit— where0644is correct — so it inherited the umask default.That file is the URL and title of every open tab. It's worse to leak than the token in the next file over: a token is revocable, browsing history isn't, and full URLs carry more than hostnames (account paths, doc links, share links with tokens in the query string).
I checked whether the directory saved us. It doesn't —
~/.config/tabbrewisdrwxr-xr-xin practice, not the0700the code intends, so the file mode was the only protection and it wasn't set. Worse with--out ./tabs.jsoninto a repo folder, where it's also agit addaway from being committed.atomicWritenow takes an optionalmode, applied to the temp file at creation so the content is never briefly readable at0644. Becauserenamemakes that temp inode become the target, the mode survives even over a pre-existing looser file — no follow-upchmodneeded, unlike the non-atomic write incredentials.ts.init's files are untouched (verified below).2. The
Origincheck didn't stop DNS rebindingThe header the check relies on isn't sent in the case that matters:
http://evil.com; attacker's DNS has a short TTLhttp://evil.com:49227/script127.0.0.1http://evil.com, so the browser treats it as same-originOriginis omitted on same-origin GET/HEAD → the check waves it throughIt gets the queued script and, because
GET /scripthas pop semantics, denies it to the real extension. Confirmed by hand before the fix:Now
Hostmust be127.0.0.1:<port>orlocalhost:<port>. The browser setsHostfrom the URL the page asked for (evil.com:49227) and page JS can't forge it —Hostis a forbidden header name. Extension traffic addresses127.0.0.1directly and is unaffected.The
Origincheck stays. Browsers always attachOriginto non-GET requests, so it's what blocks a drive-byPOST /tabsfrom writing to disk — a page can do that withContent-Type: text/plainto dodge preflight. The two checks cover different halves.Severity, honestly
Neither is urgent. The rebinding payload is a list of tab ids and group names that exists for seconds, and needs the victim on the attacker's page at exactly the right moment. The file mode matters mainly on shared machines, CI runners, and synced home directories. But the README now states a security model, and a stated model should be complete — both fixes are a handful of lines.
Test plan
bun run typecheck,bun run buildtabs.jsonis now-rw-------Host: evil.example→403 forbidden_hoston both/scriptand/health127.0.0.1andlocalhoststill200— the bridge isn't brokenPOST /tabswith a foreignOriginstill403(no regression on the check being kept)serve+pushround-trip still works end to endinitstill writesCLAUDE.md/TABBREW-CLI.md/SKILL.mdat0644🤖 Generated with Claude Code