feat(auth): inject an externally-supplied Graph access token (sandbox/CI mode) - #1
Conversation
…ction Detailed, file-and-line spec for the injected access-token mode, plus the four open decisions resolved (config reads for display only, OLK_ACCOUNT_EMAIL scope, exit code 77, env preferred over the flag).
Add OLK_ACCESS_TOKEN (plus --access-token), OLK_ACCESS_TOKEN_EXPIRES_AT, and OLK_ACCOUNT_EMAIL. When a token is supplied, GraphClient() builds a StaticTokenCredential from it and returns before touching the keyring, the account files, or the default-account config. olk never refreshes and never persists the token: its lifetime is the process's lifetime. An expired supplied expiry fails closed before any request with exit code 77 so an orchestrator can mint a fresh token and retry. With no expiry supplied, the credential reports a nominal lifetime (a zero expiry makes the azcore pipeline refuse to send) and Graph's 401 is the authority. Extract newGraphClient() so every credential source shares one construction path, keeping SetGuards (--no-write/--no-send) unbypassable. --account is rejected in token mode; --mailbox is unaffected.
auth login/logout/clean/list manage stored accounts, so they refuse to run while OLK_ACCESS_TOKEN is set. Each guard sits above the first Config()/Store() call so a refusal never opens the keyring or prompts for its password. auth status reports the injected token, its identity hint, and its expiry instead. Tests cover credential selection against a keyring that fails the test if touched, fail-closed expiry (no client is ever constructed), the exit-code mapping, an empty config dir after a token-mode run, --account rejection, --mailbox passthrough, flag-over-env precedence, guard composition, and that the token never reaches stdout, stderr, or an MCP tool result. The MCP test also pins the expired-token message, which orchestrators match on because exit codes are invisible over stdio.
Add an Access-Token Injection section under Authentication covering the three variables, the no-refresh/no-persistence semantics, exit code 77, guard composition, and the MCP lifetime caveat. List the new flags in both global-flag tables.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe CLI adds injected Microsoft Graph access-token mode through flags and environment variables, validates optional expiry, bypasses stored authentication state, preserves capability guards, restricts stored-account commands, reports token status, and returns token-specific exit codes. Documentation and comprehensive command/MCP tests are included. ChangesAccess token injection
Sequence Diagram(s)sequenceDiagram
participant CLI
participant RunContext
participant GraphClient
participant GraphAPI
CLI->>RunContext: provide injected access token and optional expiry
RunContext->>RunContext: validate token mode and bypass stored accounts
RunContext->>GraphClient: create guarded cached client
GraphClient->>GraphAPI: issue request with static token
GraphAPI-->>CLI: return command response or token error
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
SKILL.md (1)
339-339: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winWarn agents not to place bearer tokens in argv.
Add
prefer OLK_ACCESS_TOKEN; command-line arguments are visible to other processes, matchingREADME.md, so agent workflows do not expose tokens unnecessarily.internal/cmd/token_test.go (1)
298-327: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing test coverage:
AuthStatusCmdwith an already-expired token.Both
AuthStatustests here use a future expiry or no expiry hint; neither exercises an already-expired injected token. This is precisely the scenario where the current implementation (see auth.go Lines 274-284) always reportsAuthenticatedregardless of actual expiry. Once that's addressed, add a case assertingauth statussurfaces an invalid/expired status for a pastAccessTokenExpiresAt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50949a89-7a13-41e2-ad36-8771475fdebb
📒 Files selected for processing (8)
README.mdSKILL.mddocs/OLK_FORK.mddocs/OLK_FORK_STAGE2.mdinternal/cmd/auth.gointernal/cmd/root.gointernal/cmd/token.gointernal/cmd/token_test.go
| 1. Add the four fields below. | ||
| 2. Keep the same style as the other fields. Each flag has an `env` tag. | ||
| 3. Do not change the existing fields. | ||
|
|
||
| ```go | ||
| AccessToken string `help:"Delegated Graph access token; bypasses the keyring (prefer OLK_ACCESS_TOKEN)" env:"OLK_ACCESS_TOKEN" name:"access-token"` | ||
| AccessTokenExpiresAt string `help:"RFC3339 expiry of --access-token" env:"OLK_ACCESS_TOKEN_EXPIRES_AT" name:"access-token-expires-at"` | ||
| AccountEmail string `help:"Account identity hint (UPN) for use with --access-token" env:"OLK_ACCOUNT_EMAIL" name:"account-email"` | ||
| ``` | ||
|
|
||
| Notes: | ||
|
|
||
| - kong reads the `env` tag only when the command line does not give the flag. The flag wins. You write no code for this. | ||
| - `OLK_ACCOUNT` continues to set `--account`. `OLK_ACCOUNT_EMAIL` is a different variable with a different function. | ||
|
|
||
| --- | ||
|
|
||
| ## 6 Task 2 — Add the token mode helper | ||
|
|
||
| Make a new file `internal/cmd/token.go`. Put all the new logic in this file. | ||
|
|
||
| The file holds one type and three functions. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the declared counts in the implementation plan.
Line [67] says to add four fields, but the block lists only three. Line [88] says the file holds three functions, while later tasks require refuseInTokenMode and exitCodeFor plus the other helpers implemented in internal/cmd/token.go. Update these counts so the plan matches the implementation.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
2 participants
Summary
Adds an externally-supplied access-token mode to
olk: whenOLK_ACCESS_TOKENis set,olkbuilds its Graph credential from that token and never touches the OS keyring, the stored account files, or the default-account config. It never refreshes and never persists the token — the token's lifetime is the process's lifetime.This is the seam a sandboxed/CI caller needs: an external system (a token broker, a Convex backend, a CI secret) owns the OAuth flow and hands
olka short-lived delegated token, so no refresh token ever enters a disposable environment and concurrent containers can't race Microsoft's refresh-token rotation.Implements
docs/OLK_FORK_STAGE2.md(added in the first commit).Contract
OLK_ACCESS_TOKEN--access-tokenOLK_ACCESS_TOKEN_EXPIRES_AT--access-token-expires-atOLK_ACCOUNT_EMAIL--account-emailwhoamistill resolves from Graph/me.Behavior
r.Store()inGraphClient(), so the keyring,accounts/*.json, andGetDefaultAccount()are unreachable in token mode. No keyring password prompt is possible.ExpiresOnmakes the azcore pipeline refuse to send) and Graph's401is the authority.newGraphClient()is extracted so every credential source shares one construction path —SetGuards()(--no-write/--no-send) can't be bypassed by a new auth path.--no-input,--wrap-untrusted, and the command allow/deny lists are untouched.--accountis rejected (it selects among stored accounts);--mailboxdelegated access keeps working.auth login|logout|clean|listrefuse to run in token mode, each above its firstConfig()/Store()call.auth statusreports the injected token, hint, and expiry.olk mcpneeds no new plumbing — each tool call re-parses argv and therefore re-reads the environment, then rebuilds the credential per call.buildArgvnever carries the token, and the new flags are global, so they never appear in a tool's JSON schema.Authorization: <redacted>), or MCP results.Files
No changes under
internal/graphapi,internal/msauth,internal/secrets, orinternal/config— the credential interface andStaticTokenCredentialalready existed.Testing
go build ./...,go vet ./...,go mod tidy(no drift) — clean.go test -race -count=1 ./...— all packages pass.golangci-lint runat CI's pinned v2.11.4 — 0 issues.--accountrejection,--mailboxpassthrough, flag-over-env precedence, guard composition (ErrNoWrite), no-leak check on captured stdout/stderr, all fiveauthsubcommands, and an in-memory MCP session proving per-call token mode plus a stable expired-token message.OLK_CONFIG_DIRpointed at a temp dir): expired token → exit 77 with zero requests; valid-shaped injection reaches Graph and comes backInvalidAuthenticationToken(proving the keyring path was bypassed rather than erroring with "no account configured"); config dir never created;--verboseshowsAuthorization: <redacted>and zero occurrences of the sentinel token; account mode unchanged.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.