From 4bf5cb2a01c6d6d452d2f632c74a57025b917477 Mon Sep 17 00:00:00 2001 From: Ajay Date: Sun, 26 Jul 2026 22:19:24 +0000 Subject: [PATCH 1/7] docs(fork): add ASD-STE100 Stage 2 implementation plan for token injection 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). --- docs/OLK_FORK.md | 11 ++ docs/OLK_FORK_STAGE2.md | 329 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 340 insertions(+) create mode 100644 docs/OLK_FORK_STAGE2.md diff --git a/docs/OLK_FORK.md b/docs/OLK_FORK.md index 12e79bf..0827811 100644 --- a/docs/OLK_FORK.md +++ b/docs/OLK_FORK.md @@ -15,6 +15,17 @@ Stock olk has no access-token input. Its env vars (`OLK_CLIENT_ID`, `OLK_TENANT_ ## Agent prompt (copy-paste into the fork repo) +**Stage 1 is done** — this checkout is the fork (`origin PlanMonster/olkcli`, `upstream rlrghb/olkcli`, base `d3594b7`). + +**The detailed, file-and-line implementation plan for Stage 2 lives in [OLK_FORK_STAGE2.md](OLK_FORK_STAGE2.md)** (written in ASD-STE100 Simplified Technical English). Use that document as the working spec; the prompt below stays here as the contract summary and as the text to paste into an upstream-facing issue or PR description. + +Four decisions were left open by this prompt and are resolved in the Stage 2 document: + +1. Rule "no keyring or config reads" applies to account data and to all writes. `Timezone()` (`internal/cmd/root.go:161`) may still read `config.json` for display. +2. `OLK_ACCOUNT_EMAIL` is an identity hint for `auth status` and messages only. `whoami` keeps its Graph `/me` lookup. +3. The expired-token exit code is 77. +4. `--access-token` exists for parity, but the documentation prefers the environment variable, because a command line is visible to other processes. + Paste the block below verbatim as the task prompt for an agent session rooted in the `PlanMonster/olkcli` checkout. ```text diff --git a/docs/OLK_FORK_STAGE2.md b/docs/OLK_FORK_STAGE2.md new file mode 100644 index 0000000..2255cd4 --- /dev/null +++ b/docs/OLK_FORK_STAGE2.md @@ -0,0 +1,329 @@ +# Stage 2 — Access Token Injection (Implementation Plan) + +Language: ASD-STE100 Simplified Technical English. See [Writing rules](#13-writing-rules) at the end. +Parent document: [OLK_FORK.md](OLK_FORK.md). Stage 1 (fork setup) is complete. + +--- + +## 1 Scope + +This document tells the agent how to add an injected access token to `olk`. + +An external system owns the OAuth flow. That system gives a short-lived access token to `olk`. +`olk` uses the token and forgets it. `olk` does not store a refresh token in the sandbox. + +This document covers only Stage 2. It does not cover the release or the upstream pull request. + +--- + +## 2 Terms + +| Term | Meaning | +|---|---| +| Token mode | The condition when the user gives an access token to `olk`. | +| Account mode | The usual condition. `olk` reads a token from the keyring. | +| The keyring | The OS credential store. See `internal/secrets/store.go`. | +| The config dir | The directory from `config.ConfigDir()`. It holds `config.json` and `accounts/`. | +| The guards | The flags `--no-write` and `--no-send`. See `internal/graphapi/client.go:32`. | +| The sandbox | A container without a browser and without a persistent keyring. | + +--- + +## 3 Before you start + +The plan uses interfaces that are already in the fork. Confirm these five facts first. + +1. `internal/msauth/credential.go:21` has `NewStaticTokenCredential(token, expiresOn)`. +2. `internal/graphapi/client.go:66` takes an `azcore.TokenCredential`. You do not change this package. +3. `internal/cmd/root.go:105` has `RunContext.GraphClient()`. This function is the only place that makes a client. +4. `internal/cmd/root.go:68` makes the keyring store. Only `GraphClient()` and the `auth` commands get there. +5. `internal/config/paths.go:9` reads `OLK_CONFIG_DIR`. The tests use this variable. + +--- + +## 4 Design rules + +Obey these ten rules. Each rule is testable. + +1. `olk` must not read the keyring in token mode. +2. `olk` must not write the keyring in token mode. +3. `olk` must not read or write an account file in token mode. +4. `olk` must not refresh the token. +5. `olk` must not write the token to a disk file. +6. `olk` must not show the token in output, in an error, or in a log. +7. `olk` must stop before the first network call if the token is expired. +8. `olk` must keep the guards active in token mode. +9. `olk` must keep `--mailbox` operative in token mode. +10. `olk` must refuse `--account` in token mode. + +CAUTION: Rule 6 includes the process arguments. Do not put the token into an argv that `olk` prints. + +--- + +## 5 Task 1 — Add the flags + +Edit `RootFlags` in `internal/cmd/root.go:27`. + +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. + +```go +// tokenMode holds an externally supplied access token. +type tokenMode struct { + token string + expiresAt time.Time // zero when the caller gives no expiry + email string +} + +// errTokenExpired reports an expired injected token. +var errTokenExpired = errors.New("access token expired; obtain a fresh token") + +// exitTokenExpired is the exit code for errTokenExpired. +const exitTokenExpired = 77 + +// nominalTokenLifetime is the expiry that the credential reports when the +// caller gives no expiry. Graph is the true authority; a 401 is the real answer. +const nominalTokenLifetime = 30 * time.Minute +``` + +Write `newTokenMode(f *RootFlags) (*tokenMode, error)` with this behavior: + +1. Return `nil, nil` when `f.AccessToken` is empty. The caller then uses account mode. +2. Return an error when `f.Account` is not empty. Use the text `--account is not compatible with OLK_ACCESS_TOKEN`. +3. Parse `f.AccessTokenExpiresAt` with `time.RFC3339` when the field is not empty. +4. Return a parse error that names the format. Do not put the token in the message. +5. Return `errTokenExpired` when the expiry is in the past. +6. Return the `tokenMode` value. + +Write `func (t *tokenMode) credential() azcore.TokenCredential`: + +1. Use `t.expiresAt` when the value is not zero. +2. Use `time.Now().Add(nominalTokenLifetime)` when the value is zero. +3. Return `msauth.NewStaticTokenCredential(t.token, expiry)`. + +Reason for step 2: the Azure pipeline refuses a credential that reports a zero expiry. The nominal +value lets the request go to Graph. Graph then answers with 401 if the token is not valid. + +--- + +## 7 Task 3 — Add the branch in GraphClient + +Edit `RunContext.GraphClient()` in `internal/cmd/root.go:105`. + +Do this refactor first, because it prevents a bug: + +1. Move the last part of `GraphClient()` into a new method. The last part is line 138 to line 153. +2. Name the new method `newGraphClient(cred azcore.TokenCredential) (*graphapi.Client, error)`. +3. Keep the verbose selection in the new method. +4. Keep the call to `client.SetGuards(...)` in the new method. Rule 8 depends on this call. +5. Keep the cache assignment `r.client = client` in the new method. + +Then add the branch: + +```go +func (r *RunContext) GraphClient() (*graphapi.Client, error) { + if r.client != nil { + return r.client, nil + } + + // Token mode: an external system owns the OAuth flow. Do not touch the + // keyring, the account files, or the default-account config. + tm, err := newTokenMode(r.Flags) + if err != nil { + return nil, err + } + if tm != nil { + return r.newGraphClient(tm.credential()) + } + + // ... the existing account-mode code, unchanged ... +} +``` + +The branch is above the call to `r.Store()`. This position satisfies rules 1 to 4. + +--- + +## 8 Task 4 — Change the auth commands + +Edit `internal/cmd/auth.go`. The five subcommands are at line 16 to line 21. + +Add one guard function to `internal/cmd/token.go`: + +```go +// refuseInTokenMode blocks a stored-account command while a token is injected. +func refuseInTokenMode(f *RootFlags, cmd string) error { + if f.AccessToken == "" { + return nil + } + return fmt.Errorf("auth %s manages stored accounts and is unavailable when OLK_ACCESS_TOKEN is set", cmd) +} +``` + +Then make these changes. + +| Command | Change | Position | +|---|---|---| +| `auth login` | Call `refuseInTokenMode`. Return the error. | First statement, line 34. | +| `auth logout` | Call `refuseInTokenMode`. Return the error. | First statement, line 109. | +| `auth clean` | Call `refuseInTokenMode`. Return the error. | First statement, line 147. | +| `auth list` | Call `refuseInTokenMode`. Return the error. | First statement, line 214. | +| `auth status` | Report the injected token. Then return. | First statement, line 258. | + +`auth status` in token mode prints these lines and no other line: + +``` +Account: +Status: Authenticated (injected access token) +Expires: +``` + +Each guard is above the first call to `ctx.Config()` or `ctx.Store()` in the function. This position +satisfies rules 1 to 3 for the `auth` group. + +--- + +## 9 Task 5 — Add the exit code + +Edit `Execute()` in `internal/cmd/root.go:243`. + +1. Find the error block after `ctx.Run(runCtx)`. +2. Add a test for `errTokenExpired` with `errors.Is`. +3. Return `exitTokenExpired` for that error. Return 1 for all other errors. +4. Print the message to stderr with the existing `outfmt.SanitizeMultiline` call. + +```go +err := ctx.Run(runCtx) +if err != nil { + fmt.Fprintf(os.Stderr, "Error: %s\n", outfmt.SanitizeMultiline(err.Error())) + if errors.Is(err, errTokenExpired) { + return exitTokenExpired + } + return 1 +} +``` + +--- + +## 10 Task 6 — Confirm the MCP server + +The MCP server needs no new code. Confirm the four facts below. Then add the test in Task 7. + +1. `newKongParser` at `internal/cmd/mcp_server.go:200` builds the same `CLI` struct. kong reads the same `env` tags. The new flags therefore work in the server. +2. `buildArgv` at `internal/cmd/mcp_invoke.go:193` builds a small argv. The function does not add `--access-token`. Rule 6 stays true. Do not add the flag to this function. +3. The handler at `internal/cmd/mcp_invoke.go:66` makes a new `RunContext` for each call. Each call therefore gets a new client from your branch. +4. `internal/graphapi/client.go:171` redacts the `Authorization` header in verbose logs. Rule 6 stays true for `--verbose`. + +--- + +## 11 Task 7 — Write the tests + +Put the tests in `internal/cmd/token_test.go`. Use the stdlib `testing` package. +The tests are in package `cmd`, so a test can set the private `store` field of `RunContext`. + +Write these seven tests. + +### 11.1 Credential selection + +1. Make a fake `secrets.Store`. Each method calls `t.Fatal`. +2. Put the fake in `RunContext.store`. +3. Set `Flags.AccessToken`. +4. Call `GraphClient()`. +5. Confirm that the call returns a client and no error. +6. Confirm that the fake reports no access. + +### 11.2 Expiry stops the command + +1. Set `Flags.AccessTokenExpiresAt` to a time in the past. +2. Call `newTokenMode`. +3. Confirm `errors.Is(err, errTokenExpired)`. +4. Confirm that no HTTP transport runs. Use a transport that calls `t.Fatal`. + +### 11.3 Exit code + +1. Confirm that `errTokenExpired` maps to 77. +2. Test the mapping function, not the whole process. + +### 11.4 No persistence + +1. Set `OLK_CONFIG_DIR` to `t.TempDir()`. +2. Run a command in token mode against a mock transport. +3. Walk the temp directory. +4. Confirm that the directory holds no file. + +### 11.5 Flag interplay + +1. Confirm that `--account` with a token returns an error. +2. Confirm that `--mailbox` with a token passes to `resolveMailboxTarget`. +3. Confirm that `--access-token` on the command line wins against `OLK_ACCESS_TOKEN`. + +### 11.6 Auth commands + +1. Confirm that `login`, `logout`, `clean`, and `list` return an error in token mode. +2. Confirm that each error message names `OLK_ACCESS_TOKEN`. +3. Confirm that `status` prints the injected-token lines. +4. Confirm that `status` does not touch the fake store. + +### 11.7 No token in output + +1. Use a token with a unique value, for example `SENTINEL-TOKEN-VALUE`. +2. Run a command in token mode with `--verbose` and with `--dry-run`. +3. Capture stdout and stderr with `captureStd` from `internal/cmd/mcp_capture.go`. +4. Confirm that neither stream holds the sentinel value. + +--- + +## 12 Task 8 — Documentation and checks + +1. Add a section `Access-token injection` to `README.md`. Put the section under `Authentication`. +2. Describe the three environment variables in a table. +3. State that `olk` does not refresh and does not store the token. +4. Give one sandbox example with `OLK_ACCESS_TOKEN` and `--no-write`. +5. Add the same three variables to `SKILL.md` if that file lists the environment variables. +6. Run `make test`. All tests must pass. +7. Run `make lint`. The linter must report no finding. +8. Keep the diff small. Change only the files in this plan. + +Expected files in the diff: + +``` +internal/cmd/root.go (flags, branch, exit code) +internal/cmd/token.go (new) +internal/cmd/token_test.go (new) +internal/cmd/auth.go (five guards) +README.md (one section) +``` + +--- + +## 13 Writing rules + +This document follows ASD-STE100: + +- One instruction per sentence. Procedural sentences hold 20 words or fewer. +- Active voice and the imperative form in all procedures. +- Present tense only. +- No gerund as a verb. +- `must` marks a requirement. `must not` marks a prohibition. +- A caution comes before the step that it applies to. +- Code identifiers, flag names, and file paths are technical names. STE permits them. From 85e1c768912944433ebd7047d21e77ddec0cff2d Mon Sep 17 00:00:00 2001 From: Ajay Date: Sun, 26 Jul 2026 22:34:30 +0000 Subject: [PATCH 2/7] feat(auth): accept an externally supplied Graph access token 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. --- internal/cmd/root.go | 30 ++++++++- internal/cmd/token.go | 137 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 internal/cmd/token.go diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 294947e..7312e5c 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -8,6 +8,7 @@ import ( "time" _ "time/tzdata" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/alecthomas/kong" "github.com/rlrghb/olkcli/internal/config" @@ -46,6 +47,14 @@ type RootFlags struct { NoInput bool `help:"Fail instead of prompting (headless/agent safety)" env:"OLK_NO_INPUT"` WrapUntrusted bool `help:"Wrap external free-text in untrusted-content markers (JSON/plain output)" env:"OLK_WRAP_UNTRUSTED"` + // Injected access token. An external system owns the OAuth flow and supplies a + // short-lived delegated token; olk then never touches the keyring, never + // refreshes, and never persists the token. Prefer the environment variables: + // a command line is visible to other processes on the host. + AccessToken string `help:"Delegated Graph access token; bypasses the keyring entirely (prefer OLK_ACCESS_TOKEN)" env:"OLK_ACCESS_TOKEN" name:"access-token"` + AccessTokenExpiresAt string `help:"RFC3339 expiry of the injected access token; a past value fails before any request" env:"OLK_ACCESS_TOKEN_EXPIRES_AT" name:"access-token-expires-at"` + AccountEmail string `help:"Account identity hint (UPN) used for display when an access token is injected" env:"OLK_ACCOUNT_EMAIL" name:"account-email"` + // Command-scoping allow/deny lists (comma-separated dotted paths). EnableCommands string `help:"Allow only these command prefixes (csv; e.g. mail,calendar)" env:"OLK_ENABLE_COMMANDS"` EnableCommandsExact string `help:"Allow only these exact command paths (csv; e.g. mail.list,mail.get)" env:"OLK_ENABLE_COMMANDS_EXACT"` @@ -107,6 +116,17 @@ func (r *RunContext) GraphClient() (*graphapi.Client, error) { return r.client, nil } + // Token mode short-circuits account resolution: an external system supplied + // the credential, so the keyring, the account files, and the default-account + // config are never touched. This branch must stay above r.Store(). + tm, err := newTokenMode(r.Flags) + if err != nil { + return nil, err + } + if tm != nil { + return r.newGraphClient(tm.credential()) + } + store, err := r.Store() if err != nil { return nil, err @@ -135,7 +155,15 @@ func (r *RunContext) GraphClient() (*graphapi.Client, error) { return nil, fmt.Errorf("getting credentials: %w", err) } + return r.newGraphClient(cred) +} + +// newGraphClient builds, guards, and caches the Graph client for a credential. +// Every credential source funnels through here, so a new authentication path can +// never accidentally bypass the capability guards. +func (r *RunContext) newGraphClient(cred azcore.TokenCredential) (*graphapi.Client, error) { var client *graphapi.Client + var err error if r.Flags.Verbose { client, err = graphapi.NewClientVerbose(cred) } else { @@ -243,7 +271,7 @@ func Execute() int { err := ctx.Run(runCtx) if err != nil { fmt.Fprintf(os.Stderr, "Error: %s\n", outfmt.SanitizeMultiline(err.Error())) - return 1 + return exitCodeFor(err) } return 0 } diff --git a/internal/cmd/token.go b/internal/cmd/token.go new file mode 100644 index 0000000..9574b72 --- /dev/null +++ b/internal/cmd/token.go @@ -0,0 +1,137 @@ +package cmd + +import ( + "errors" + "fmt" + "strings" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + + "github.com/rlrghb/olkcli/internal/msauth" +) + +// Token mode: an external system (CI, an agent sandbox, a token broker) owns the +// OAuth flow and hands olk a short-lived delegated access token. olk then acts +// as a pure consumer of that token: +// +// - it never reads or writes the OS keyring or the stored account files, +// - it never refreshes, and never persists the token anywhere, +// - the token's lifetime is the process's lifetime. +// +// This keeps long-lived refresh tokens outside disposable environments, where a +// container-local keyring would both leak a durable credential and race +// Microsoft's refresh-token rotation across concurrent containers. + +// errTokenExpired reports an injected access token whose supplied expiry has +// already passed. It is a sentinel so Execute can map it to a distinct exit +// code and callers can re-mint a token without parsing message text. +var errTokenExpired = errors.New("access token expired; obtain a fresh token") + +// exitTokenExpired is the process exit code for errTokenExpired. It is distinct +// from the generic failure code (1) so an orchestrator can react by minting a +// fresh token instead of treating the run as a hard error. +const exitTokenExpired = 77 + +// nominalTokenLifetime is the expiry reported to the Azure SDK when the caller +// supplies a token without an expiry. A zero expiry would make the SDK's +// credential pipeline treat the token as already expired and refuse to send the +// request, so we report a plausible remaining lifetime and let Graph be the +// authority: a stale token comes back as 401, which olk already maps to its +// unauthenticated error class. +const nominalTokenLifetime = 30 * time.Minute + +// tokenMode holds an externally supplied access token and its optional +// metadata. A nil *tokenMode means account mode (the usual keyring-backed path). +type tokenMode struct { + token string + expiresAt time.Time // zero when the caller supplied no expiry + email string // optional identity hint (OLK_ACCOUNT_EMAIL) +} + +// newTokenMode returns the injected-token configuration, or nil when no token is +// supplied. It fails closed: an unparseable or already-passed expiry is an error +// raised before any network call and before any credential store is touched. +func newTokenMode(f *RootFlags) (*tokenMode, error) { + if f == nil { + return nil, nil + } + token := strings.TrimSpace(f.AccessToken) + if token == "" { + return nil, nil + } + + // --account selects among *stored* accounts, which token mode does not read. + // Failing here is clearer than silently ignoring the flag. + if strings.TrimSpace(f.Account) != "" { + return nil, errors.New("--account is not compatible with OLK_ACCESS_TOKEN; use --mailbox for delegated access, or OLK_ACCOUNT_EMAIL as an identity hint") + } + + tm := &tokenMode{token: token, email: strings.TrimSpace(f.AccountEmail)} + + if raw := strings.TrimSpace(f.AccessTokenExpiresAt); raw != "" { + exp, err := time.Parse(time.RFC3339, raw) + if err != nil { + // Report the expected format, never the value alongside the token. + return nil, errors.New("invalid access token expiry: expected RFC3339, e.g. 2026-01-01T12:34:56Z") + } + tm.expiresAt = exp + if !time.Now().Before(exp) { + return nil, errTokenExpired + } + } + + return tm, nil +} + +// credential builds the azcore credential for the injected token. The Graph +// wrapper accepts any azcore.TokenCredential, so nothing below this point knows +// or cares where the token came from. +func (t *tokenMode) credential() azcore.TokenCredential { + expiry := t.expiresAt + if expiry.IsZero() { + expiry = time.Now().Add(nominalTokenLifetime) + } + return msauth.NewStaticTokenCredential(t.token, expiry) +} + +// accountLabel returns a display identity for the injected token. Token mode has +// no stored account metadata, so it uses the caller's hint when present. +func (t *tokenMode) accountLabel() string { + if t.email != "" { + return t.email + } + return "unknown (injected token)" +} + +// expiryLabel renders the supplied expiry for display, or "unknown" when the +// caller did not supply one. +func (t *tokenMode) expiryLabel() string { + if t.expiresAt.IsZero() { + return "unknown" + } + return t.expiresAt.UTC().Format(time.RFC3339) +} + +// exitCodeFor maps a command error to a process exit code. Everything is a +// generic failure (1) except an expired injected token, which gets its own code +// so an orchestrator can mint a fresh token and retry. +func exitCodeFor(err error) int { + if err == nil { + return 0 + } + if errors.Is(err, errTokenExpired) { + return exitTokenExpired + } + return 1 +} + +// refuseInTokenMode blocks a command that manages stored accounts while a token +// is injected. Call it before any keyring or config access so a refusal never +// prompts for a keyring password. +func refuseInTokenMode(f *RootFlags, cmd string) error { + if f == nil || strings.TrimSpace(f.AccessToken) == "" { + return nil + } + return fmt.Errorf("auth %s manages stored accounts and is unavailable when OLK_ACCESS_TOKEN is set", cmd) +} From 06dbe81c698e9d6ba8aef051a5a2e596455222ff Mon Sep 17 00:00:00 2001 From: Ajay Date: Sun, 26 Jul 2026 23:47:38 +0000 Subject: [PATCH 3/7] feat(auth): make auth subcommands token-mode aware, add tests 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. --- internal/cmd/auth.go | 27 +++ internal/cmd/token_test.go | 388 +++++++++++++++++++++++++++++++++++++ 2 files changed, 415 insertions(+) create mode 100644 internal/cmd/token_test.go diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 94550ec..61e5242 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -31,6 +31,10 @@ type AuthLoginCmd struct { } func (c *AuthLoginCmd) Run(ctx *RunContext) error { + if err := refuseInTokenMode(ctx.Flags, "login"); err != nil { + return err + } + clientID := c.ClientID if clientID == "" { clientID = config.DefaultClientID @@ -106,6 +110,10 @@ type AuthLogoutCmd struct { } func (c *AuthLogoutCmd) Run(ctx *RunContext) error { + if err := refuseInTokenMode(ctx.Flags, "logout"); err != nil { + return err + } + cfg, err := ctx.Config() if err != nil { return err @@ -144,6 +152,10 @@ func (c *AuthLogoutCmd) Run(ctx *RunContext) error { type AuthCleanCmd struct{} func (c *AuthCleanCmd) Run(ctx *RunContext) error { + if err := refuseInTokenMode(ctx.Flags, "clean"); err != nil { + return err + } + if !ctx.Flags.Force { return fmt.Errorf("this will remove ALL stored accounts and tokens; use --force to confirm") } @@ -211,6 +223,10 @@ func (c *AuthCleanCmd) Run(ctx *RunContext) error { type AuthListCmd struct{} func (c *AuthListCmd) Run(ctx *RunContext) error { + if err := refuseInTokenMode(ctx.Flags, "list"); err != nil { + return err + } + cfg, err := ctx.Config() if err != nil { return err @@ -255,6 +271,17 @@ func (c *AuthListCmd) Run(ctx *RunContext) error { type AuthStatusCmd struct{} func (c *AuthStatusCmd) Run(ctx *RunContext) error { + // Token mode has no stored account to inspect: report the injected credential + // instead, without touching the keyring or the config. + if tm, err := newTokenMode(ctx.Flags); err != nil { + return err + } else if tm != nil { + fmt.Printf("Account: %s\n", outfmt.Sanitize(tm.accountLabel())) + fmt.Println("Status: Authenticated (injected access token)") + fmt.Printf("Expires: %s\n", tm.expiryLabel()) + return nil + } + cfg, err := ctx.Config() if err != nil { return err diff --git a/internal/cmd/token_test.go b/internal/cmd/token_test.go new file mode 100644 index 0000000..92259c0 --- /dev/null +++ b/internal/cmd/token_test.go @@ -0,0 +1,388 @@ +package cmd + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/rlrghb/olkcli/internal/graphapi" + "github.com/rlrghb/olkcli/internal/secrets" +) + +// sentinelToken is deliberately unique so a leak test can grep for it. +const sentinelToken = "SENTINEL-ACCESS-TOKEN-DO-NOT-PRINT" + +// policyOpts are the token request options the Graph SDK would pass. +func policyOpts() policy.TokenRequestOptions { + return policy.TokenRequestOptions{Scopes: []string{"https://graph.microsoft.com/.default"}} +} + +// failStore is a secrets.Store that fails the test if anything touches it. It +// proves token mode never reaches the keyring: no reads, no writes, no prompts. +type failStore struct{ t *testing.T } + +func (s failStore) Set(key, value string) error { + s.t.Fatalf("keyring Set(%q) called in token mode", key) + return nil +} + +func (s failStore) Get(key string) (string, error) { + s.t.Fatalf("keyring Get(%q) called in token mode", key) + return "", nil +} + +func (s failStore) Delete(key string) error { + s.t.Fatalf("keyring Delete(%q) called in token mode", key) + return nil +} + +func (s failStore) Keys() ([]string, error) { + s.t.Fatalf("keyring Keys() called in token mode") + return nil, nil +} + +// tokenRunContext returns a RunContext wired to a keyring that fails on use and +// to a throwaway config dir, so any stored-credential access is a test failure. +func tokenRunContext(t *testing.T, flags *RootFlags) *RunContext { + t.Helper() + t.Setenv("OLK_CONFIG_DIR", t.TempDir()) + return &RunContext{Ctx: context.Background(), Flags: flags, store: failStore{t}} +} + +// --- 11.1 credential selection ---------------------------------------------- + +func TestTokenMode_SelectsStaticCredentialWithoutKeyring(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken}) + + client, err := ctx.GraphClient() + if err != nil { + t.Fatalf("GraphClient in token mode: %v", err) + } + if client == nil { + t.Fatal("expected a Graph client") + } + // A second call must reuse the cached client (still no keyring access). + if again, err := ctx.GraphClient(); err != nil || again != client { + t.Fatalf("expected cached client, got %v, %v", again, err) + } +} + +func TestTokenMode_GuardsStillApply(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken, NoWrite: true, NoSend: true}) + + client, err := ctx.GraphClient() + if err != nil { + t.Fatalf("GraphClient: %v", err) + } + // The guards live on the client, so exercising any mutating method proves + // token mode did not bypass SetGuards. CreateDraft refuses before any I/O. + if _, err := client.CreateDraft(ctx.Ctx, "s", "b", []string{"a@b.com"}, nil, nil, false); !errors.Is(err, graphapi.ErrNoWrite) { + t.Fatalf("expected ErrNoWrite in token mode, got %v", err) + } +} + +func TestAccountMode_UnchangedWhenNoTokenSupplied(t *testing.T) { + if tm, err := newTokenMode(&RootFlags{}); err != nil || tm != nil { + t.Fatalf("expected account mode (nil, nil), got %v, %v", tm, err) + } + if tm, err := newTokenMode(&RootFlags{AccessToken: " "}); err != nil || tm != nil { + t.Fatalf("whitespace-only token must mean account mode, got %v, %v", tm, err) + } +} + +// --- 11.2 expiry fails closed ------------------------------------------------ + +func TestTokenMode_ExpiredFailsClosed(t *testing.T) { + past := time.Now().Add(-time.Minute).UTC().Format(time.RFC3339) + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken, AccessTokenExpiresAt: past}) + + _, err := ctx.GraphClient() + if !errors.Is(err, errTokenExpired) { + t.Fatalf("expected errTokenExpired, got %v", err) + } + // No client was constructed, so no request could have been sent. + if ctx.client != nil { + t.Fatal("a Graph client was built for an expired token") + } +} + +func TestTokenMode_FutureExpiryIsUsedVerbatim(t *testing.T) { + want := time.Now().Add(2 * time.Hour).UTC().Truncate(time.Second) + tm, err := newTokenMode(&RootFlags{ + AccessToken: sentinelToken, + AccessTokenExpiresAt: want.Format(time.RFC3339), + }) + if err != nil { + t.Fatalf("newTokenMode: %v", err) + } + tok, err := tm.credential().GetToken(context.Background(), policyOpts()) + if err != nil { + t.Fatalf("GetToken: %v", err) + } + if !tok.ExpiresOn.UTC().Equal(want) { + t.Fatalf("expiry = %s, want %s", tok.ExpiresOn.UTC(), want) + } + if tok.Token != sentinelToken { + t.Fatal("credential returned a different token than the one supplied") + } +} + +func TestTokenMode_MissingExpiryGetsNominalLifetime(t *testing.T) { + tm, err := newTokenMode(&RootFlags{AccessToken: sentinelToken}) + if err != nil { + t.Fatalf("newTokenMode: %v", err) + } + tok, err := tm.credential().GetToken(context.Background(), policyOpts()) + if err != nil { + t.Fatalf("GetToken: %v", err) + } + // A zero expiry would make the azcore pipeline treat the token as expired + // and refuse to send the request, so the credential must report a future one. + if !tok.ExpiresOn.After(time.Now().Add(nominalTokenLifetime - time.Minute)) { + t.Fatalf("expected a nominal future expiry, got %s", tok.ExpiresOn) + } +} + +func TestTokenMode_MalformedExpiryIsRejectedWithoutEchoingToken(t *testing.T) { + _, err := newTokenMode(&RootFlags{AccessToken: sentinelToken, AccessTokenExpiresAt: "yesterday"}) + if err == nil { + t.Fatal("expected an error for a malformed expiry") + } + if !strings.Contains(err.Error(), "RFC3339") { + t.Fatalf("error should name the expected format, got %q", err) + } + if strings.Contains(err.Error(), sentinelToken) { + t.Fatal("error message leaked the access token") + } +} + +// --- 11.3 exit code ---------------------------------------------------------- + +func TestExitCodeFor(t *testing.T) { + if got := exitCodeFor(nil); got != 0 { + t.Fatalf("nil error => %d, want 0", got) + } + if got := exitCodeFor(errors.New("boom")); got != 1 { + t.Fatalf("generic error => %d, want 1", got) + } + if got := exitCodeFor(errTokenExpired); got != exitTokenExpired { + t.Fatalf("expired token => %d, want %d", got, exitTokenExpired) + } + // Wrapped sentinels must keep the distinct code so callers can retry. + wrapped := errors.Join(errors.New("context"), errTokenExpired) + if got := exitCodeFor(wrapped); got != exitTokenExpired { + t.Fatalf("wrapped expired token => %d, want %d", got, exitTokenExpired) + } +} + +// --- 11.4 no persistence ----------------------------------------------------- + +func TestTokenMode_WritesNothingToConfigDir(t *testing.T) { + dir := t.TempDir() + t.Setenv("OLK_CONFIG_DIR", dir) + + flags := &RootFlags{AccessToken: sentinelToken, AccountEmail: "user@corp.com"} + ctx := &RunContext{Ctx: context.Background(), Flags: flags, store: failStore{t}} + if _, err := ctx.GraphClient(); err != nil { + t.Fatalf("GraphClient: %v", err) + } + if _, _, err := captureStd(func() error { return (&AuthStatusCmd{}).Run(ctx) }); err != nil { + t.Fatalf("auth status: %v", err) + } + + var found []string + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if path != dir { + found = append(found, path) + } + return nil + }) + if err != nil { + t.Fatalf("walking config dir: %v", err) + } + if len(found) != 0 { + t.Fatalf("token mode created config state: %v", found) + } +} + +// --- 11.5 flag interplay ----------------------------------------------------- + +func TestTokenMode_AccountFlagIsRejected(t *testing.T) { + _, err := newTokenMode(&RootFlags{AccessToken: sentinelToken, Account: "someone@corp.com"}) + if err == nil || !strings.Contains(err.Error(), "OLK_ACCESS_TOKEN") { + t.Fatalf("expected --account to be refused in token mode, got %v", err) + } +} + +func TestTokenMode_MailboxStillResolves(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken, Mailbox: "shared@corp.com"}) + if _, err := ctx.GraphClient(); err != nil { + t.Fatalf("GraphClient with --mailbox: %v", err) + } + target, err := resolveMailboxTarget(ctx.Flags.Mailbox) + if err != nil { + t.Fatalf("resolveMailboxTarget: %v", err) + } + if target != "shared@corp.com" { + t.Fatalf("mailbox target = %q, want shared@corp.com", target) + } +} + +func TestTokenMode_FlagBeatsEnv(t *testing.T) { + t.Setenv("OLK_ACCESS_TOKEN", "env-token") + t.Setenv("OLK_ACCESS_TOKEN_EXPIRES_AT", "") + t.Setenv("OLK_ACCOUNT", "") + + cli := &CLI{} + k, err := newKongParser(cli) + if err != nil { + t.Fatalf("newKongParser: %v", err) + } + if _, err := k.Parse([]string{"--access-token", "flag-token", "version"}); err != nil { + t.Fatalf("Parse: %v", err) + } + if cli.AccessToken != "flag-token" { + t.Fatalf("AccessToken = %q, want flag-token", cli.AccessToken) + } + + cli2 := &CLI{} + k2, err := newKongParser(cli2) + if err != nil { + t.Fatalf("newKongParser: %v", err) + } + if _, err := k2.Parse([]string{"version"}); err != nil { + t.Fatalf("Parse: %v", err) + } + if cli2.AccessToken != "env-token" { + t.Fatalf("AccessToken = %q, want env-token", cli2.AccessToken) + } +} + +// --- 11.6 auth subcommands --------------------------------------------------- + +func TestAuthCommands_RefusedInTokenMode(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken, Force: true}) + + cases := map[string]func() error{ + "login": func() error { return (&AuthLoginCmd{}).Run(ctx) }, + "logout": func() error { return (&AuthLogoutCmd{}).Run(ctx) }, + "clean": func() error { return (&AuthCleanCmd{}).Run(ctx) }, + "list": func() error { return (&AuthListCmd{}).Run(ctx) }, + } + for name, run := range cases { + t.Run(name, func(t *testing.T) { + _, _, err := captureStd(run) + if err == nil { + t.Fatalf("auth %s must fail in token mode", name) + } + if !strings.Contains(err.Error(), "OLK_ACCESS_TOKEN") { + t.Fatalf("auth %s error should name OLK_ACCESS_TOKEN, got %q", name, err) + } + if !strings.Contains(err.Error(), name) { + t.Fatalf("auth %s error should name the command, got %q", name, err) + } + }) + } +} + +func TestAuthStatus_ReportsInjectedToken(t *testing.T) { + expiry := time.Now().Add(time.Hour).UTC().Truncate(time.Second) + ctx := tokenRunContext(t, &RootFlags{ + AccessToken: sentinelToken, + AccountEmail: "user@corp.com", + AccessTokenExpiresAt: expiry.Format(time.RFC3339), + }) + + out, _, err := captureStd(func() error { return (&AuthStatusCmd{}).Run(ctx) }) + if err != nil { + t.Fatalf("auth status: %v", err) + } + for _, want := range []string{"user@corp.com", "injected access token", expiry.Format(time.RFC3339)} { + if !strings.Contains(out, want) { + t.Fatalf("auth status output missing %q:\n%s", want, out) + } + } +} + +func TestAuthStatus_UnknownIdentityWithoutHint(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{AccessToken: sentinelToken}) + + out, _, err := captureStd(func() error { return (&AuthStatusCmd{}).Run(ctx) }) + if err != nil { + t.Fatalf("auth status: %v", err) + } + if !strings.Contains(out, "unknown (injected token)") || !strings.Contains(out, "Expires: unknown") { + t.Fatalf("unexpected auth status output:\n%s", out) + } +} + +// --- 11.7 the token never appears in output ---------------------------------- + +func TestTokenMode_TokenNeverPrinted(t *testing.T) { + ctx := tokenRunContext(t, &RootFlags{ + AccessToken: sentinelToken, + AccountEmail: "user@corp.com", + Verbose: true, + DryRun: true, + }) + + out, errOut, err := captureStd(func() error { + if _, cerr := ctx.GraphClient(); cerr != nil { + return cerr + } + return (&AuthStatusCmd{}).Run(ctx) + }) + if err != nil { + t.Fatalf("run: %v", err) + } + if strings.Contains(out, sentinelToken) || strings.Contains(errOut, sentinelToken) { + t.Fatalf("access token leaked into output:\nstdout: %s\nstderr: %s", out, errOut) + } +} + +// --- token mode under the MCP server ---------------------------------------- + +// The MCP server re-parses argv (and therefore re-reads the environment) on every +// tool call, so token mode must apply per call without any MCP-specific plumbing. +func TestMCP_HonorsTokenMode(t *testing.T) { + t.Setenv("OLK_CONFIG_DIR", t.TempDir()) + t.Setenv("OLK_ACCOUNT", "") + t.Setenv("OLK_ACCESS_TOKEN", sentinelToken) + t.Setenv("OLK_ACCESS_TOKEN_EXPIRES_AT", time.Now().Add(-time.Minute).UTC().Format(time.RFC3339)) + + cs := connectE2E(t) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + res, err := cs.CallTool(ctx, &mcp.CallToolParams{Name: "whoami"}) + if err != nil { + t.Fatalf("CallTool: %v", err) + } + text := resultText(res) + // The expired-token failure must surface as stable, matchable text: an + // orchestrator keys off it to mint a fresh token (exit codes are invisible + // over MCP). + if !strings.Contains(text, "access token expired") { + t.Fatalf("expected the expired-token message, got: %s", text) + } + if strings.Contains(text, sentinelToken) { + t.Fatal("access token leaked into an MCP tool result") + } + // Reaching the token-mode branch means no stored account was consulted. + if strings.Contains(text, "no account configured") { + t.Fatal("MCP tool call fell through to account mode") + } +} + +// compile-time assertion: failStore satisfies the keyring interface it stands in for. +var _ secrets.Store = failStore{} From fd84034141e1957f7336ee16445f6d3420fd95b6 Mon Sep 17 00:00:00 2001 From: Ajay Date: Sun, 26 Jul 2026 23:47:38 +0000 Subject: [PATCH 4/7] docs: document access-token injection 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. --- README.md | 41 ++++++++++++++++++++++++++++++++++++++++- SKILL.md | 3 +++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 97b7566..f5f6e89 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,42 @@ olk auth login --browser [--client-id YOUR_CLIENT_ID --tenant-id YOUR_TENANT_ID] This flow requires the app registration to have a loopback redirect URI (see [Custom App Registration](#custom-app-registration) step 4). If the browser shows error **AADSTS50011** ("redirect URI mismatch") and the CLI times out, the app registration is missing the redirect URI — use `--client-id` with your own registration. +### Access-Token Injection (CI, containers, agent sandboxes) + +When an external system already owns the OAuth flow, hand `olk` a short-lived **delegated Graph access token** instead of logging in. In this mode `olk` is a pure consumer of that token: + +- it **never** reads or writes the OS keyring, the account files, or the default-account config; +- it **never** refreshes and **never** persists the token — the token's lifetime is the process's lifetime; +- no refresh token ever enters the environment, so a disposable container holds nothing durable. + +| Variable | Required | Description | +|----------|----------|-------------| +| `OLK_ACCESS_TOKEN` | yes | A delegated Microsoft Graph access token (bearer). Also available as `--access-token`, but prefer the variable: a command line is visible to other processes. | +| `OLK_ACCESS_TOKEN_EXPIRES_AT` | no | RFC3339 expiry of that token (e.g. `2026-01-01T12:34:56Z`). A past value fails **before** any network call with exit code **77**, so a caller can mint a fresh token and retry. Omit it and Graph's `401` is the authority instead. | +| `OLK_ACCOUNT_EMAIL` | no | Identity hint (UPN) shown by `olk auth status`. `olk whoami` still resolves the real profile from Graph. | + +```bash +# In a CI job or sandbox: read-only, no prompts, no stored credentials. +export OLK_ACCESS_TOKEN="$(fetch-token-from-your-broker)" +export OLK_ACCESS_TOKEN_EXPIRES_AT="2026-01-01T12:34:56Z" +export OLK_NO_WRITE=1 OLK_NO_INPUT=1 + +olk mail list --json +olk auth status +# Account: user@example.com +# Status: Authenticated (injected access token) +# Expires: 2026-01-01T12:34:56Z +``` + +The token must carry the Graph scopes for the commands you run (the same scopes `olk auth login` requests). Every capability guard composes normally — `--no-write`, `--no-send`, `--no-input`, `--wrap-untrusted`, `--enable-commands[-exact]`, `--disable-commands` — and `--mailbox` delegated access keeps working. + +Notes: + +- `--account` selects among *stored* accounts, so it is rejected in this mode. Use `--mailbox` for delegated access, or `OLK_ACCOUNT_EMAIL` as a display hint. +- `olk auth login|logout|clean|list` manage stored accounts and refuse to run while `OLK_ACCESS_TOKEN` is set. `olk auth status` reports the injected token. +- Under `olk mcp`, every tool call rebuilds the credential from the environment, so the same variables apply with no extra configuration. Because the token is not refreshed, keep the server's lifetime shorter than the token's. +- The token never appears in output, errors, verbose HTTP logs (the `Authorization` header is redacted), or the argv the MCP server builds. + ### Custom App Registration If your organization blocks the default client ID, or your admin requires apps to be registered under your tenant, you'll need to create your own app registration: @@ -338,7 +374,7 @@ For common workflows, `olk` provides top-level shortcuts: |------|---------|-------------| | `--json` | `OLK_JSON` | JSON output | | `--plain` | `OLK_PLAIN` | TSV output | -| `--account EMAIL` | `OLK_ACCOUNT` | Account to use | +| `--account EMAIL` | `OLK_ACCOUNT` | Account to use (stored accounts only) | | `--mailbox EMAIL` | `OLK_MAILBOX` | Target another user's mailbox via delegated access (requires `Mail.Read.Shared` at login) | | `-v, --verbose` | `OLK_VERBOSE` | Verbose output | | `--dry-run` | `OLK_DRY_RUN` | Dry run mode | @@ -355,6 +391,9 @@ For common workflows, `olk` provides top-level shortcuts: | `--enable-commands CSV` | `OLK_ENABLE_COMMANDS` | Allow only these command prefixes (e.g. `mail,calendar`) | | `--enable-commands-exact CSV` | `OLK_ENABLE_COMMANDS_EXACT` | Allow only these exact command paths (e.g. `mail.list,mail.get`) | | `--disable-commands CSV` | `OLK_DISABLE_COMMANDS` | Block these command paths (overrides allows) | +| `--access-token TOKEN` | `OLK_ACCESS_TOKEN` | Use an injected Graph access token; bypasses the keyring entirely ([details](#access-token-injection-ci-containers-agent-sandboxes)) | +| `--access-token-expires-at TS` | `OLK_ACCESS_TOKEN_EXPIRES_AT` | RFC3339 expiry of the injected token; a past value exits 77 before any request | +| `--account-email EMAIL` | `OLK_ACCOUNT_EMAIL` | Identity hint (UPN) displayed when a token is injected | | | `OLK_KEYRING_PASSWORD` | File-backend keyring password (for headless use) | These capability guards apply to **every** entry path — the bare CLI, scripts/CI, and the MCP server — so `OLK_NO_WRITE=1 olk --mailbox boss@example.com mail list` is a hard "read this mailbox, never write" guarantee. diff --git a/SKILL.md b/SKILL.md index 843a7ed..df408b4 100644 --- a/SKILL.md +++ b/SKILL.md @@ -336,6 +336,9 @@ export OLK_MAILBOX=boss@example.com | `--color auto\|never\|always` | `OLK_COLOR` | Color mode | | `--timeout SECONDS` | `OLK_TIMEOUT` | Request timeout (default 60) | | `--tz TIMEZONE` | `OLK_TIMEZONE` | IANA time zone for display (e.g. `America/New_York`) | +| `--access-token TOKEN` | `OLK_ACCESS_TOKEN` | Use an injected Graph access token instead of a stored login: no keyring, no refresh, no persistence. `--account` is rejected in this mode; `--mailbox` still works | +| `--access-token-expires-at TS` | `OLK_ACCESS_TOKEN_EXPIRES_AT` | RFC3339 expiry of the injected token. A past value exits **77** before any request — mint a fresh token and retry | +| `--account-email EMAIL` | `OLK_ACCOUNT_EMAIL` | Identity hint (UPN) shown by `olk auth status` when a token is injected | ## Capability Guards (CLI, MCP, and scripts) From 0b013998ebe305ef35a5aedd472b0abbb011d41a Mon Sep 17 00:00:00 2001 From: Ajay Bhargava Date: Sun, 26 Jul 2026 19:52:05 -0400 Subject: [PATCH 5/7] Delete docs/OLK_FORK.md --- docs/OLK_FORK.md | 133 ----------------------------------------------- 1 file changed, 133 deletions(-) delete mode 100644 docs/OLK_FORK.md diff --git a/docs/OLK_FORK.md b/docs/OLK_FORK.md deleted file mode 100644 index 0827811..0000000 --- a/docs/OLK_FORK.md +++ /dev/null @@ -1,133 +0,0 @@ -# olk Fork: Token Injection, Release, Upstream PR - -Owns: the fork of `rlrghb/olkcli`, the agent prompt to implement the change in that repo, the release process that makes the binary installable in agent containers, and the upstream PR strategy. Snapshot-side installation is owned by [SNAPSHOT_PACKAGING.md](SNAPSHOT_PACKAGING.md). - -## Why the fork exists - -Stock olk has no access-token input. Its env vars (`OLK_CLIENT_ID`, `OLK_TENANT_ID`, `OLK_KEYRING_PASSWORD`, `OLK_ACCOUNT`) all serve its own login/keyring flows. The only headless path seeds a refresh token into a container-local keyring, which violates refresh-token containment and races Microsoft's refresh-token rotation across concurrent Daytona containers. The seam already exists upstream: `internal/msauth` has a static token credential and `internal/graphapi` accepts any `azcore.TokenCredential`. The fork adds an env route into that seam and nothing else. - -## Fork setup - -1. Fork `github.com/rlrghb/olkcli` to `github.com/PlanMonster/olkcli`. -2. Branch `access-token-injection` off the latest upstream release tag (record which one). -3. Apply the change below via the agent prompt. -4. Tag fork releases as `v-nanoclaw.` (for example `v0.9.1-nanoclaw.1`) so the upstream base is always readable from the version. - -## Agent prompt (copy-paste into the fork repo) - -**Stage 1 is done** — this checkout is the fork (`origin PlanMonster/olkcli`, `upstream rlrghb/olkcli`, base `d3594b7`). - -**The detailed, file-and-line implementation plan for Stage 2 lives in [OLK_FORK_STAGE2.md](OLK_FORK_STAGE2.md)** (written in ASD-STE100 Simplified Technical English). Use that document as the working spec; the prompt below stays here as the contract summary and as the text to paste into an upstream-facing issue or PR description. - -Four decisions were left open by this prompt and are resolved in the Stage 2 document: - -1. Rule "no keyring or config reads" applies to account data and to all writes. `Timezone()` (`internal/cmd/root.go:161`) may still read `config.json` for display. -2. `OLK_ACCOUNT_EMAIL` is an identity hint for `auth status` and messages only. `whoami` keeps its Graph `/me` lookup. -3. The expired-token exit code is 77. -4. `--access-token` exists for parity, but the documentation prefers the environment variable, because a command line is visible to other processes. - -Paste the block below verbatim as the task prompt for an agent session rooted in the `PlanMonster/olkcli` checkout. - -```text -Implement an externally-supplied access-token credential mode for olk. This must be -upstream-quality, generic (no company-specific naming), and narrowly scoped. Do not -change any other behavior. - -## Contract - -Three new environment variables, honored by every CLI command and by `olk mcp`: - - OLK_ACCESS_TOKEN A delegated Microsoft Graph access token (bearer). - OLK_ACCESS_TOKEN_EXPIRES_AT Optional RFC3339 expiry for that token. - OLK_ACCOUNT_EMAIL Optional account identity hint (UPN/email) used where - olk would otherwise read stored account metadata. - -Also add a --access-token flag on the root command mirroring OLK_ACCESS_TOKEN -(flag wins over env), consistent with how other global flags pair with env vars. - -## Required behavior (all must hold) - -1. When OLK_ACCESS_TOKEN is set, olk MUST NOT touch the OS keyring, the encrypted - file store, or stored account metadata: no reads, no writes, no prompts. Token - mode bypasses account resolution entirely. -2. Construct the credential from the supplied token. internal/msauth already has a - static token credential (used in tests); reuse or promote it. The Graph client in - internal/graphapi accepts azcore.TokenCredential, so no Graph-layer changes - should be needed beyond wiring. -3. Never refresh. Never persist the token anywhere (no config dir writes, no cache - files). The token's lifecycle is the process's lifecycle. -4. If OLK_ACCESS_TOKEN_EXPIRES_AT is set and in the past, fail closed BEFORE any - network call, with a clear stderr message ("access token expired; obtain a fresh - token") and a distinct nonzero exit code. If unset, pass the token through and - let Graph return 401 naturally; map that 401 to the same "unauthenticated" error - class olk already uses. -5. Identity: `olk whoami` and any code path that needs the signed-in account's - email should use OLK_ACCOUNT_EMAIL when present, else resolve via Graph /me. - --account must error in token mode ("--account is not compatible with - OLK_ACCESS_TOKEN"); --mailbox (delegated mailbox targeting) must keep working. -6. `olk auth login|logout|clean|list|status` in token mode: status should report - that an injected token is in use (and its expiry if known); the others should - error with a message explaining that auth commands manage stored accounts and - are unavailable when OLK_ACCESS_TOKEN is set. -7. All existing guard flags must compose unchanged with token mode: --no-write, - --no-send, --no-input, --wrap-untrusted, --enable-commands[-exact], - --disable-commands, and the OLK_MCP_* controls for `olk mcp`. -8. The token must never appear in logs, verbose output, error messages, or dry-run - output. - -## Where to look - -- internal/msauth: credential construction, static token credential, device-code - and browser flows (do not modify the flows; add a selection branch before them). -- internal/cmd: root command setup, global flag/env binding, account resolution. -- internal/secrets and internal/config: the code paths that must be provably - skipped in token mode. -- internal/graphapi: confirm the client takes azcore.TokenCredential and needs no - change. - -## Tests (required) - -- Credential selection: OLK_ACCESS_TOKEN set -> static credential chosen; keyring - and config account lookup never invoked (inject a fake keyring that fails the - test if touched). -- Expiry: past OLK_ACCESS_TOKEN_EXPIRES_AT fails closed with the documented exit - code and no network call (use a transport that fails the test if used). -- Persistence: after running a command in token mode against a mocked Graph - transport, the config dir contains no new/modified files. -- Flag interplay: --account errors; --mailbox passes through; --access-token flag - overrides env. -- auth subcommand behavior per item 6. -- `olk mcp` honors token mode identically (spawn the server with the env set and a - mocked transport; verify a read tool call works and no keyring access happens). - -## Docs - -Add a README section "Access-token injection" under Authentication describing the -three variables, the no-refresh/no-persistence semantics, and the intended use -case (an external system owns OAuth and hands short-lived tokens to olk in CI or -agent sandboxes). Keep the existing security posture language. - -Run make test and make lint before finishing. Keep the diff reviewable: one -feature, no drive-by refactors. -``` - -## Release process (makes the binary installable in containers) - -The upstream repo ships `.goreleaser.yaml` and a Makefile, so the fork can release real binaries with no new infrastructure: - -1. Verify the fork has a release workflow (upstream builds via GoReleaser on tag push; if the workflow is not present in the fork, add a minimal `.github/workflows/release.yml` that runs `goreleaser release --clean` on `v*` tags with `GITHUB_TOKEN`). -2. Tag and push: `git tag v0.9.1-nanoclaw.1 && git push origin v0.9.1-nanoclaw.1`. -3. GoReleaser publishes GitHub release assets including `olk__linux_amd64.tar.gz` and a `checksums.txt`. -4. Record two values for the consumer side: the release tag and the sha256 of the linux/amd64 asset. These become the `OLK_VERSION` / `OLK_SHA256` pins in `scripts/daytona-snapshot.ts` (see [SNAPSHOT_PACKAGING.md](SNAPSHOT_PACKAGING.md)). - -Fallback if GoReleaser is unavailable: `make build` in a linux/amd64 environment and attach `bin/olk` to a manually created GitHub release, plus a sha256. Do not consume unpinned artifacts (no `@latest`, no branch tarballs). - -Do not publish to npm from the fork; the upstream `olkcli` npm name is not ours, and the snapshot consumes a checksummed binary directly. - -## Upstream PR - -Open the PR against `rlrghb/olkcli` from the same branch before NanoClaw work builds on the fork (the feature is generic and matches olk's existing env-pairing conventions, so it has a real chance of landing). The plan does not depend on it merging. If it merges, repoint the snapshot pin at the upstream release and retire the fork. - -## Version bump procedure - -When upstream releases a new version we want: rebase `access-token-injection` onto the new tag, re-run the fork test suite, tag `v-nanoclaw.1`, update the two pins in `scripts/daytona-snapshot.ts`, and let the snapshot content hash roll the Daytona image. From 13d13c0103274bb21a89c176746022085d5be5d9 Mon Sep 17 00:00:00 2001 From: Ajay Bhargava Date: Sun, 26 Jul 2026 19:52:34 -0400 Subject: [PATCH 6/7] Delete docs/OLK_FORK_STAGE2.md --- docs/OLK_FORK_STAGE2.md | 329 ---------------------------------------- 1 file changed, 329 deletions(-) delete mode 100644 docs/OLK_FORK_STAGE2.md diff --git a/docs/OLK_FORK_STAGE2.md b/docs/OLK_FORK_STAGE2.md deleted file mode 100644 index 2255cd4..0000000 --- a/docs/OLK_FORK_STAGE2.md +++ /dev/null @@ -1,329 +0,0 @@ -# Stage 2 — Access Token Injection (Implementation Plan) - -Language: ASD-STE100 Simplified Technical English. See [Writing rules](#13-writing-rules) at the end. -Parent document: [OLK_FORK.md](OLK_FORK.md). Stage 1 (fork setup) is complete. - ---- - -## 1 Scope - -This document tells the agent how to add an injected access token to `olk`. - -An external system owns the OAuth flow. That system gives a short-lived access token to `olk`. -`olk` uses the token and forgets it. `olk` does not store a refresh token in the sandbox. - -This document covers only Stage 2. It does not cover the release or the upstream pull request. - ---- - -## 2 Terms - -| Term | Meaning | -|---|---| -| Token mode | The condition when the user gives an access token to `olk`. | -| Account mode | The usual condition. `olk` reads a token from the keyring. | -| The keyring | The OS credential store. See `internal/secrets/store.go`. | -| The config dir | The directory from `config.ConfigDir()`. It holds `config.json` and `accounts/`. | -| The guards | The flags `--no-write` and `--no-send`. See `internal/graphapi/client.go:32`. | -| The sandbox | A container without a browser and without a persistent keyring. | - ---- - -## 3 Before you start - -The plan uses interfaces that are already in the fork. Confirm these five facts first. - -1. `internal/msauth/credential.go:21` has `NewStaticTokenCredential(token, expiresOn)`. -2. `internal/graphapi/client.go:66` takes an `azcore.TokenCredential`. You do not change this package. -3. `internal/cmd/root.go:105` has `RunContext.GraphClient()`. This function is the only place that makes a client. -4. `internal/cmd/root.go:68` makes the keyring store. Only `GraphClient()` and the `auth` commands get there. -5. `internal/config/paths.go:9` reads `OLK_CONFIG_DIR`. The tests use this variable. - ---- - -## 4 Design rules - -Obey these ten rules. Each rule is testable. - -1. `olk` must not read the keyring in token mode. -2. `olk` must not write the keyring in token mode. -3. `olk` must not read or write an account file in token mode. -4. `olk` must not refresh the token. -5. `olk` must not write the token to a disk file. -6. `olk` must not show the token in output, in an error, or in a log. -7. `olk` must stop before the first network call if the token is expired. -8. `olk` must keep the guards active in token mode. -9. `olk` must keep `--mailbox` operative in token mode. -10. `olk` must refuse `--account` in token mode. - -CAUTION: Rule 6 includes the process arguments. Do not put the token into an argv that `olk` prints. - ---- - -## 5 Task 1 — Add the flags - -Edit `RootFlags` in `internal/cmd/root.go:27`. - -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. - -```go -// tokenMode holds an externally supplied access token. -type tokenMode struct { - token string - expiresAt time.Time // zero when the caller gives no expiry - email string -} - -// errTokenExpired reports an expired injected token. -var errTokenExpired = errors.New("access token expired; obtain a fresh token") - -// exitTokenExpired is the exit code for errTokenExpired. -const exitTokenExpired = 77 - -// nominalTokenLifetime is the expiry that the credential reports when the -// caller gives no expiry. Graph is the true authority; a 401 is the real answer. -const nominalTokenLifetime = 30 * time.Minute -``` - -Write `newTokenMode(f *RootFlags) (*tokenMode, error)` with this behavior: - -1. Return `nil, nil` when `f.AccessToken` is empty. The caller then uses account mode. -2. Return an error when `f.Account` is not empty. Use the text `--account is not compatible with OLK_ACCESS_TOKEN`. -3. Parse `f.AccessTokenExpiresAt` with `time.RFC3339` when the field is not empty. -4. Return a parse error that names the format. Do not put the token in the message. -5. Return `errTokenExpired` when the expiry is in the past. -6. Return the `tokenMode` value. - -Write `func (t *tokenMode) credential() azcore.TokenCredential`: - -1. Use `t.expiresAt` when the value is not zero. -2. Use `time.Now().Add(nominalTokenLifetime)` when the value is zero. -3. Return `msauth.NewStaticTokenCredential(t.token, expiry)`. - -Reason for step 2: the Azure pipeline refuses a credential that reports a zero expiry. The nominal -value lets the request go to Graph. Graph then answers with 401 if the token is not valid. - ---- - -## 7 Task 3 — Add the branch in GraphClient - -Edit `RunContext.GraphClient()` in `internal/cmd/root.go:105`. - -Do this refactor first, because it prevents a bug: - -1. Move the last part of `GraphClient()` into a new method. The last part is line 138 to line 153. -2. Name the new method `newGraphClient(cred azcore.TokenCredential) (*graphapi.Client, error)`. -3. Keep the verbose selection in the new method. -4. Keep the call to `client.SetGuards(...)` in the new method. Rule 8 depends on this call. -5. Keep the cache assignment `r.client = client` in the new method. - -Then add the branch: - -```go -func (r *RunContext) GraphClient() (*graphapi.Client, error) { - if r.client != nil { - return r.client, nil - } - - // Token mode: an external system owns the OAuth flow. Do not touch the - // keyring, the account files, or the default-account config. - tm, err := newTokenMode(r.Flags) - if err != nil { - return nil, err - } - if tm != nil { - return r.newGraphClient(tm.credential()) - } - - // ... the existing account-mode code, unchanged ... -} -``` - -The branch is above the call to `r.Store()`. This position satisfies rules 1 to 4. - ---- - -## 8 Task 4 — Change the auth commands - -Edit `internal/cmd/auth.go`. The five subcommands are at line 16 to line 21. - -Add one guard function to `internal/cmd/token.go`: - -```go -// refuseInTokenMode blocks a stored-account command while a token is injected. -func refuseInTokenMode(f *RootFlags, cmd string) error { - if f.AccessToken == "" { - return nil - } - return fmt.Errorf("auth %s manages stored accounts and is unavailable when OLK_ACCESS_TOKEN is set", cmd) -} -``` - -Then make these changes. - -| Command | Change | Position | -|---|---|---| -| `auth login` | Call `refuseInTokenMode`. Return the error. | First statement, line 34. | -| `auth logout` | Call `refuseInTokenMode`. Return the error. | First statement, line 109. | -| `auth clean` | Call `refuseInTokenMode`. Return the error. | First statement, line 147. | -| `auth list` | Call `refuseInTokenMode`. Return the error. | First statement, line 214. | -| `auth status` | Report the injected token. Then return. | First statement, line 258. | - -`auth status` in token mode prints these lines and no other line: - -``` -Account: -Status: Authenticated (injected access token) -Expires: -``` - -Each guard is above the first call to `ctx.Config()` or `ctx.Store()` in the function. This position -satisfies rules 1 to 3 for the `auth` group. - ---- - -## 9 Task 5 — Add the exit code - -Edit `Execute()` in `internal/cmd/root.go:243`. - -1. Find the error block after `ctx.Run(runCtx)`. -2. Add a test for `errTokenExpired` with `errors.Is`. -3. Return `exitTokenExpired` for that error. Return 1 for all other errors. -4. Print the message to stderr with the existing `outfmt.SanitizeMultiline` call. - -```go -err := ctx.Run(runCtx) -if err != nil { - fmt.Fprintf(os.Stderr, "Error: %s\n", outfmt.SanitizeMultiline(err.Error())) - if errors.Is(err, errTokenExpired) { - return exitTokenExpired - } - return 1 -} -``` - ---- - -## 10 Task 6 — Confirm the MCP server - -The MCP server needs no new code. Confirm the four facts below. Then add the test in Task 7. - -1. `newKongParser` at `internal/cmd/mcp_server.go:200` builds the same `CLI` struct. kong reads the same `env` tags. The new flags therefore work in the server. -2. `buildArgv` at `internal/cmd/mcp_invoke.go:193` builds a small argv. The function does not add `--access-token`. Rule 6 stays true. Do not add the flag to this function. -3. The handler at `internal/cmd/mcp_invoke.go:66` makes a new `RunContext` for each call. Each call therefore gets a new client from your branch. -4. `internal/graphapi/client.go:171` redacts the `Authorization` header in verbose logs. Rule 6 stays true for `--verbose`. - ---- - -## 11 Task 7 — Write the tests - -Put the tests in `internal/cmd/token_test.go`. Use the stdlib `testing` package. -The tests are in package `cmd`, so a test can set the private `store` field of `RunContext`. - -Write these seven tests. - -### 11.1 Credential selection - -1. Make a fake `secrets.Store`. Each method calls `t.Fatal`. -2. Put the fake in `RunContext.store`. -3. Set `Flags.AccessToken`. -4. Call `GraphClient()`. -5. Confirm that the call returns a client and no error. -6. Confirm that the fake reports no access. - -### 11.2 Expiry stops the command - -1. Set `Flags.AccessTokenExpiresAt` to a time in the past. -2. Call `newTokenMode`. -3. Confirm `errors.Is(err, errTokenExpired)`. -4. Confirm that no HTTP transport runs. Use a transport that calls `t.Fatal`. - -### 11.3 Exit code - -1. Confirm that `errTokenExpired` maps to 77. -2. Test the mapping function, not the whole process. - -### 11.4 No persistence - -1. Set `OLK_CONFIG_DIR` to `t.TempDir()`. -2. Run a command in token mode against a mock transport. -3. Walk the temp directory. -4. Confirm that the directory holds no file. - -### 11.5 Flag interplay - -1. Confirm that `--account` with a token returns an error. -2. Confirm that `--mailbox` with a token passes to `resolveMailboxTarget`. -3. Confirm that `--access-token` on the command line wins against `OLK_ACCESS_TOKEN`. - -### 11.6 Auth commands - -1. Confirm that `login`, `logout`, `clean`, and `list` return an error in token mode. -2. Confirm that each error message names `OLK_ACCESS_TOKEN`. -3. Confirm that `status` prints the injected-token lines. -4. Confirm that `status` does not touch the fake store. - -### 11.7 No token in output - -1. Use a token with a unique value, for example `SENTINEL-TOKEN-VALUE`. -2. Run a command in token mode with `--verbose` and with `--dry-run`. -3. Capture stdout and stderr with `captureStd` from `internal/cmd/mcp_capture.go`. -4. Confirm that neither stream holds the sentinel value. - ---- - -## 12 Task 8 — Documentation and checks - -1. Add a section `Access-token injection` to `README.md`. Put the section under `Authentication`. -2. Describe the three environment variables in a table. -3. State that `olk` does not refresh and does not store the token. -4. Give one sandbox example with `OLK_ACCESS_TOKEN` and `--no-write`. -5. Add the same three variables to `SKILL.md` if that file lists the environment variables. -6. Run `make test`. All tests must pass. -7. Run `make lint`. The linter must report no finding. -8. Keep the diff small. Change only the files in this plan. - -Expected files in the diff: - -``` -internal/cmd/root.go (flags, branch, exit code) -internal/cmd/token.go (new) -internal/cmd/token_test.go (new) -internal/cmd/auth.go (five guards) -README.md (one section) -``` - ---- - -## 13 Writing rules - -This document follows ASD-STE100: - -- One instruction per sentence. Procedural sentences hold 20 words or fewer. -- Active voice and the imperative form in all procedures. -- Present tense only. -- No gerund as a verb. -- `must` marks a requirement. `must not` marks a prohibition. -- A caution comes before the step that it applies to. -- Code identifiers, flag names, and file paths are technical names. STE permits them. From ef712283eb3ba4fd71f4235053fd4925162df940 Mon Sep 17 00:00:00 2001 From: ajay-bhargava Date: Sun, 26 Jul 2026 23:56:15 +0000 Subject: [PATCH 7/7] docs: use future placeholder for access-token expiry examples Co-authored-by: Codesmith --- README.md | 6 +++--- internal/cmd/token.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f5f6e89..e3ce3eb 100644 --- a/README.md +++ b/README.md @@ -218,20 +218,20 @@ When an external system already owns the OAuth flow, hand `olk` a short-lived ** | Variable | Required | Description | |----------|----------|-------------| | `OLK_ACCESS_TOKEN` | yes | A delegated Microsoft Graph access token (bearer). Also available as `--access-token`, but prefer the variable: a command line is visible to other processes. | -| `OLK_ACCESS_TOKEN_EXPIRES_AT` | no | RFC3339 expiry of that token (e.g. `2026-01-01T12:34:56Z`). A past value fails **before** any network call with exit code **77**, so a caller can mint a fresh token and retry. Omit it and Graph's `401` is the authority instead. | +| `OLK_ACCESS_TOKEN_EXPIRES_AT` | no | RFC3339 expiry of that token (e.g. `2030-01-01T12:34:56Z`). A past value fails **before** any network call with exit code **77**, so a caller can mint a fresh token and retry. Omit it and Graph's `401` is the authority instead. | | `OLK_ACCOUNT_EMAIL` | no | Identity hint (UPN) shown by `olk auth status`. `olk whoami` still resolves the real profile from Graph. | ```bash # In a CI job or sandbox: read-only, no prompts, no stored credentials. export OLK_ACCESS_TOKEN="$(fetch-token-from-your-broker)" -export OLK_ACCESS_TOKEN_EXPIRES_AT="2026-01-01T12:34:56Z" +export OLK_ACCESS_TOKEN_EXPIRES_AT="2030-01-01T12:34:56Z" export OLK_NO_WRITE=1 OLK_NO_INPUT=1 olk mail list --json olk auth status # Account: user@example.com # Status: Authenticated (injected access token) -# Expires: 2026-01-01T12:34:56Z +# Expires: 2030-01-01T12:34:56Z ``` The token must carry the Graph scopes for the commands you run (the same scopes `olk auth login` requests). Every capability guard composes normally — `--no-write`, `--no-send`, `--no-input`, `--wrap-untrusted`, `--enable-commands[-exact]`, `--disable-commands` — and `--mailbox` delegated access keeps working. diff --git a/internal/cmd/token.go b/internal/cmd/token.go index 9574b72..ab32b2a 100644 --- a/internal/cmd/token.go +++ b/internal/cmd/token.go @@ -73,7 +73,7 @@ func newTokenMode(f *RootFlags) (*tokenMode, error) { exp, err := time.Parse(time.RFC3339, raw) if err != nil { // Report the expected format, never the value alongside the token. - return nil, errors.New("invalid access token expiry: expected RFC3339, e.g. 2026-01-01T12:34:56Z") + return nil, errors.New("invalid access token expiry: expected RFC3339, e.g. 2030-01-01T12:34:56Z") } tm.expiresAt = exp if !time.Now().Before(exp) {