The wrapper prints and posts review service text as is, and follows its redirects to other origins #52
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The wrapper treats everything the pinned service origin returns as safe. It prints and posts service text without removing the capability token, and it follows a redirect from the service to any other origin. Both matter only if the service, or something in front of it, repeats the token or sends a redirect, and the wrapper has no defense if that happens.
What happens
Service text is printed.
toFailurebuilds each mapped failure from the service's own words (src/review/session.ts:43-86), including the rejection body for HTTP 401 and 403 (src/review/session.ts:48-53).describeError(src/wrapper/orchestrator.ts:153-162). When the body is not a problem document, that is the whole raw body.src/wrapper/orchestrator.ts:93-98).Service text is posted to the pull request.
src/wrapper/orchestrator.ts:47-55,src/reconcile/summary.ts:16-18).src/reconcile/inline.ts:14-27,src/reconcile/inline.ts:83).Redirects are followed. The bundled
@j4k/reviewclient callsfetchwith noredirectoption (dist/index.mjs:5749), andcreateCapabilityClienttakes only a URL and a token (src/review/session.ts:197-199), so the wrapper cannot change that. The origin pin (src/credentials.ts:61-66) and thehttpscheck (src/credentials.ts:40) cover the configured URL and nothing after a redirect. The runtime drops theAuthorizationheader on a cross-origin redirect, but it still sends the request, and the wrapper parses and reconciles the answer.Reproduced at
9f272a52with the committed bundle on Node 24.21.0 against a stand-in service on loopback:s1-reflect-401);subject_commitrepeats the token: it appeared in the summary comment, the inline review, or the log line (s1-echo-report,s1-claim-body-echo,s1-success-field);s1-redirect-xorigin). When theLocationURL carried the token, the token reached the other origin in the URL (s1-redirect-token-in-url).What it should do instead
src/credentials.tsis the only module that reads the token, andsrc/main.tsis the only module that may import it (src/credentials.test.ts:209-214). So the function should live incredentials.tsand reach the print and post sites throughWrapperDeps, asopenSessiondoes.@j4k/reviewtransport; the wrapper then picks it up by bumping the dependency (package.json:30) and rebuilding the bundle.Why it matters
A token in a pull request comment is visible to everyone who can read the pull request, which on a public repository is everyone. No runner masks comments. A token that reaches another origin in a URL ends up in that origin's logs, over plain http if the redirect says so. The capability token has no expiry.
The trigger is a service bug, a misconfigured proxy, or a hijacked origin that echoes the
Authorizationheader or redirects. The reproductions used a stand-in service that does so. Nobody has checked whether the deployed service, or a proxy in front of it, ever repeats the credential or redirects.Related: #23 covers how a claim body renders inside an inline comment (footnotes, deep nesting). This issue is about what service text may contain, such as the token, and what happens to it.
🤖 Generated with Claude Code