fix(addon): harden token bridge and deliver the passphrase to the add-on (#953)#975
Open
aaspinwall wants to merge 1 commit into
Open
fix(addon): harden token bridge and deliver the passphrase to the add-on (#953)#975aaspinwall wants to merge 1 commit into
aaspinwall wants to merge 1 commit into
Conversation
…d-on Security: the token-bridge content script forwards window.postMessages straight to the privileged background (OIDC tokens, encryption passphrase, sign-in/out). It ran on <all_urls> with its sender checks commented out. It now validates e.source === window and e.origin === window.location.origin, and content_scripts.matches is scoped to the Send app origins. build.sh strips the http://localhost/* match for production builds so a released add-on never trusts a localhost page. Also removes a dead debug "visual cue" block. Functional: only the management page (useSendConfig) pulled the bridged passphrase from browser.storage.local into the keychain, so a key restore driven from the popup or background could find no passphrase. Extract pullBridgedPassphrase() and call it inside restoreKeysUsingLocalStorage — which both the popup (PopupView) and background (initCloudFile) already call — so neither depends on the management page having run the transfer first. useSendConfig now delegates to the same helper. Adds a unit test for it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b4b38fa to
763f831
Compare
f100b75 to
fe60f63
Compare
Collaborator
Author
|
CodeQL note (for reviewers): the one red CodeQL check flags
Suggest merging on the green functional checks and tracking the cleartext alert via #982 rather than blocking here. |
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.
What changed?
Hardening of the add-on "token bridge" — the content script that relays messages from the Send web app to the privileged background (OIDC tokens, the encryption passphrase, sign-in/out).
1. Bridge security (closes #953):
token-bridge.jsforwardedwindow.postMessages straight to the background. It ran on<all_urls>with its sender checks commented out. It now rejects messages unlesse.source === windowande.origin === window.location.origin.manifest.jsoncontent_scripts.matchesscoped to the app origins (send.tb.pro,send-stage.tb.pro,localhost);build.shstripshttp://localhost/*from production builds so a released add-on never trusts a localhost page. Removed a dead debug block.2. Passphrase delivery to popup/background (see #712):
pullBridgedPassphrase()and call it insiderestoreKeysUsingLocalStorage(which the popup and background already call), so neither depends on the management page having run the transfer.AI disclosure: implemented with Claude Code (an AI agent) under my direction and review; reviewed by independent agent passes.
Why?
The bridge relayed secrets to the privileged background but trusted any page/frame and injected everywhere (#953), and the shared passphrase didn't reach the upload popup (#712).
Scope note
Passphrase encryption at rest was originally bundled here but has been split into its own PR — it's incompatible with the dev e2e's cross-session
storageStatereuse (which can't carry the IndexedDB key) and needs a dedicated e2e rework. This PR contains only the bridge hardening + delivery, which don't change the passphrase storage format.Verification
Frontend unit suite green;
tscclean for changed files; bridge origin-scoping +pullBridgedPassphrasecovered by unit tests and real-browser checks. e2e is unaffected by these changes (no storage-format change).Applicable Issues
Closes #953
Related: #712