Problem
This milestone adds real, persistent, full-write-access GitHub credential storage server-side (#[persist-token issue]) plus a new retrieval endpoint and a refresh/expiration strategy (#[refresh issue]) — genuinely security-sensitive infrastructure change, not routine feature work. Deserves an explicit review pass before/alongside shipping, not just "did the tests pass."
Area
Security review, cross-cutting over the other issues in this milestone.
Proposal
Once #[persist-token issue] and #[refresh issue] land, do a focused review pass covering:
- Encryption at rest is actually correct (not just "a
btoa call") and keys are managed the same way other secrets in this codebase are.
- No path (logs, audit events, error messages, telemetry, the MCP usage-tracking
recordProductUsageEvent calls, Sentry captures if any) ever includes the raw token.
- Logout/session-revocation genuinely deletes the stored token, verified by test, not just assumed from the code path.
- The new token-retrieval endpoint is unreachable by anything other than a real session identity (re-verify against the static "mcp"/"api" identities explicitly).
- If refresh-token rotation was chosen in #[refresh issue]: refresh tokens themselves need the same at-rest protection as access tokens, not weaker.
- Rate-limit/abuse posture on the new token-retrieval endpoint specifically (a session that can repeatedly mint fresh tokens is a different risk profile than one that can't).
Deliverables
Resources
- #[persist-token issue], #[refresh issue], #[cli-ams issue]
Boundaries
This issue's job is to review, not to re-derive the design from scratch. Maintainer-only for now.
maintainer-only — assigned JSONbored.
Problem
This milestone adds real, persistent, full-write-access GitHub credential storage server-side (#[persist-token issue]) plus a new retrieval endpoint and a refresh/expiration strategy (#[refresh issue]) — genuinely security-sensitive infrastructure change, not routine feature work. Deserves an explicit review pass before/alongside shipping, not just "did the tests pass."
Area
Security review, cross-cutting over the other issues in this milestone.
Proposal
Once #[persist-token issue] and #[refresh issue] land, do a focused review pass covering:
btoacall") and keys are managed the same way other secrets in this codebase are.recordProductUsageEventcalls, Sentry captures if any) ever includes the raw token.Deliverables
Resources
Boundaries
This issue's job is to review, not to re-derive the design from scratch. Maintainer-only for now.
maintainer-only — assigned JSONbored.