fix: split API/WS auth tokens and add rotation/revocation#122
Conversation
Summary of ChangesHello @teng-lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's security posture by overhauling its authentication token management. It introduces a robust system for generating, rotating, and revoking distinct tokens for API and WebSocket communication, moving away from a single shared token. This change improves the resilience against token compromise and provides more granular control over access, ensuring that the application adheres to modern security best practices. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
4d96e44 to
ca7bfd2
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a significant security improvement by splitting API and WebSocket authentication tokens and implementing rotation, expiry, and revocation for them. The new RotatingTokenAuthority class is well-designed and tested. The changes are extensive, touching both server-side and client-side code, and include backward compatibility considerations. However, I've identified a high-severity issue where API calls from older clients will fail after a server update, due to the reuse of the beamcode-consumer-token meta tag for a new purpose. My review includes a detailed comment on this issue.
| document.querySelector<HTMLMetaElement>('meta[name="beamcode-api-token"]')?.content ?? | ||
| // Backward-compatible fallback for pages that only inject the legacy meta tag. | ||
| document.querySelector<HTMLMetaElement>('meta[name="beamcode-consumer-token"]')?.content ?? | ||
| null |
There was a problem hiding this comment.
This fallback logic correctly handles a new client working with an old server. However, the reverse scenario—an old client with a new server—appears to be broken.
An old client's JavaScript will only look for meta[name="beamcode-consumer-token"] for API authentication. The new server implementation now places the WebSocket-only token in this meta tag. Consequently, all API calls from old clients will fail with 401 Unauthorized errors after the server is updated, as they will be sending the wrong token.
To maintain backward compatibility for API calls, beamcode-consumer-token should continue to hold a token valid for the API. A new meta tag, for example <meta name="beamcode-ws-token">, should be introduced for the WebSocket token. This would require changes in src/http/consumer-html.ts to inject the tags correctly and in web/src/ws.ts to read from this new tag.
There was a problem hiding this comment.
Good catch. Fixed in c981f7d: beamcode-consumer-token is API-compatible again for legacy clients, and a dedicated beamcode-ws-token now carries the scoped WS token. Updated web/src/ws.ts to prefer beamcode-ws-token with legacy fallback, and added tests for this behavior.
c981f7d to
f169ce8
Compare
Summary
Addresses architecture review finding in
docs/review/architecture-review-2026-02-22.md:This PR separates API and WebSocket token scopes, adds token lifetime/rotation controls, and revokes tokens on shutdown.
What Changed
RotatingTokenAuthority:revokeAll()supportsrc/bin/beamcode.ts):src/http/server.ts):apiKeyValidator) in addition to static key modesrc/server/api-key-authenticator.ts):src/http/consumer-html.ts):beamcode-api-tokenandbeamcode-consumer-tokenbeamcode-api-tokenwith legacy fallback tobeamcode-consumer-tokentokenquery parameter frombeamcode-consumer-tokenValidation
Passed locally:
pnpm test -- src/server/rotating-token-authority.test.ts src/server/api-key-authenticator.test.ts src/http/server.test.ts src/http/consumer-html.test.tspnpm --dir web test -- src/api.test.ts src/ws.test.tspnpm typecheckCompatibility Notes