Support headless auth token storage#1127
Open
peyton-alt wants to merge 12 commits into
Open
Conversation
peyton-alt
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds explicit non-keyring authentication options for headless/container/CI environments by introducing environment- and file-based token sources, while keeping the OS keyring as the default. It also updates auth-related commands to be token-source-aware to avoid accidentally deleting or revoking credentials that aren’t locally managed.
Changes:
- Add
ENTIRE_AUTH_TOKEN(read-only, highest priority) andENTIRE_SECRETS_PATH(opt-in JSON file store) as additional auth token sources. - Make
login,logout,auth status, andauth revokesource-aware (env vs file vs keyring), and add/extend unit + integration coverage. - Document the new environment variables and precedence rules in
README.md.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents headless/CI auth variables and storage precedence. |
| cmd/entire/cli/logout.go | Updates logout to use source-aware token lookup and avoid env-token deletion/revocation. |
| cmd/entire/cli/logout_test.go | Adds coverage ensuring env tokens are not revoked or locally deleted by logout. |
| cmd/entire/cli/login.go | Refactors login to inject token store and adds headless guidance/warnings on save. |
| cmd/entire/cli/login_test.go | Adds unit tests for headless save failure hint and env-token shadowing warning. |
| cmd/entire/cli/integration_test/login_test.go | Adds integration test verifying file-backed token persistence and permissions. |
| cmd/entire/cli/auth/store.go | Extends auth store to read from env/file/keyring with explicit precedence and adds GetTokenInfo. |
| cmd/entire/cli/auth/store_test.go | Adds tests for env precedence and file store behaviors (including perms + malformed JSON). |
| cmd/entire/cli/auth/file_store.go | Introduces JSON file token store implementation with permission checks and atomic writes. |
| cmd/entire/cli/auth/env.go | Adds env var constants plus TokenSource/TokenInfo types. |
| cmd/entire/cli/auth.go | Updates auth status and revoke flows to use source-aware token info and messaging. |
| cmd/entire/cli/auth_test.go | Adds tests validating source labeling and env-token-specific status/revoke behaviors. |
Contributor
Author
|
@BugBot review |
Mirror the non-env logout branch: 401 from the server means the token was already invalid, other errors warn-and-continue. Both still print the unset-guidance so a CI re-run isn't blocked by a transient failure. Per Copilot review on PR #1127. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 462f68820c0d
- describeTokenSource: TokenInfo.Path is always set when source is file (configuredSecretsPath only returns ok=true with a non-empty path), so the empty-path fallback was unreachable. - tokenStore comment: backend can now be keyring, file, or env — clarify the abstraction rather than scoping it to keyring + a caller list. - runLogout fall-through comment: "stored token" is backend-neutral. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 95b31c3a77c9
The standalone TestRunAuthRevoke_CurrentEnvTokenRevokesButDoesNotDelete duplicated 80% of the env-source revoke harness already used by the TestRunAuthRevoke_CurrentEnvTokenServerErrors table. Adding "success" as a third table case (revokeErr: nil) keeps every assertion the standalone made (revokeByID not called, revokeCurrent gets the right token, output mentions the env var, lower-priority store untouched) while sharing the setup. Net: -32 LOC, same coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 89e4cbb6497a
Contributor
Author
|
@BugBot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b739cbe. Configure here.
Previously a transient revoke failure was downgraded to a warning while stdout still claimed the token had been revoked and the command exited 0. Env-tokens have no local copy to fall back on, so a 5xx/timeout leaves the bearer token active server-side while the user (or CI) believes it was invalidated. Now: only 401 is treated as idempotent success. Any other error returns non-zero and suppresses the "Revoked" success line. Per Codex adversarial review of #1127 (high severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: bcb0590b5df5
Previously the env token was returned for any baseURL, so a stray ENTIRE_API_BASE_URL=https://staging override (or accidental local-dev override left from another session) would silently send the production bearer to the wrong endpoint. The file/keyring stores are already keyed by baseURL — only the env path was unscoped. Now: ENTIRE_AUTH_TOKEN only applies when baseURL == api.DefaultBaseURL. Custom origins fall through to the per-origin stores. README updated. Per Codex adversarial review of #1127 (medium severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 9bdf2cb85e3a
Restores the per-invocation opt-out the original issue asked for. Previously the only way to bypass keyring was the ambient env var ENTIRE_SECRETS_PATH; if that var was unset (or stripped from the environment), 'entire login' would silently fall back to the OS keyring even when the user had explicitly opted out of it. Now: --no-keyring preflights ENTIRE_SECRETS_PATH before kicking off device auth and refuses to proceed without it. Read paths are unchanged — the flag only constrains where login writes. Per Codex adversarial review of #1127 (medium severity), and addresses the third option from the original issue request. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: cc14ee7c7659
Two concurrent CLI invocations sharing ENTIRE_SECRETS_PATH could each read the pre-write JSON, mutate different baseURL entries, and the last temp+rename would clobber the first save. atomic-rename writes make individual writes torn-free for readers, but the read-modify-write cycle was unprotected. Wraps saveFileToken / deleteFileToken in a sidecar-flock-based critical section. The credentials file itself can't be flocked safely because writeFileStore replaces the inode via temp+rename, so the lock lives on credentials.json.lock alongside it. Reads stay lock-free since atomic rename guarantees reader sees a complete prior or new file. Unix uses syscall.Flock; Windows is currently a no-op (Credential Manager via the keyring backend is the recommended path on Windows; LockFileEx hook is documented for future work). Concurrency test: 16 goroutines saving distinct keys all land in the final file with no losses, exercised under -race. Per Codex adversarial review of #1127 (medium severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a111d61d40ac
…updates" This reverts commit 19b26af. Scope reduction for #1036. The lost-update race the lock prevented is hypothetical: it requires two concurrent CLI invocations sharing one ENTIRE_SECRETS_PATH file, which is exotic in practice (CI jobs use distinct paths or ENTIRE_AUTH_TOKEN; dev machines run the CLI one at a time). 138 lines of cross-platform locking machinery + sidecar .lock convention + concurrency test is more than the threat model justifies in this PR. The atomic temp+rename writes still guarantee readers never see torn content. If concurrent-write collisions become a real reported issue, a follow-up can restore the flock without affecting the public API. Per Codex adversarial review (medium severity, deferred to follow-up). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
eek sorry for mentioning your PR in my forked repo @peyton-alt . I was showing codex this as an example..and it mentioned it in a PR |
…torage # Conflicts: # cmd/entire/cli/auth/store.go # cmd/entire/cli/integration_test/login_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/307
Summary
Adds explicit non-keyring CLI auth paths for headless, container, and CI environments. Implements all three options from issue #1036.
ENTIRE_AUTH_TOKEN— read-only env-var bearer for non-interactive workflows. Scoped to the production API origin (https://entire.io) so a strayENTIRE_API_BASE_URLoverride can't leak a prod token to a staging/dev endpoint.ENTIRE_SECRETS_PATH— opt-in JSON file-backed store. Plaintext,0600perms enforced on read, atomic temp+rename writes, refuses relative paths and group-readable files.--no-keyring— fail-closed login flag. PreflightsENTIRE_SECRETS_PATHbefore device auth and refuses to fall back to the OS keyring if no non-keyring backend is configured.Keeps OS keyring as the default backend. No automatic plaintext fallback.
Source-aware command behavior
auth status,auth list,auth revoke, andlogoutnow look up the token viaGetTokenInfoand behave correctly per source:auth statusreports provenance (e.g. "supplied by ENTIRE_AUTH_TOKEN" vs "stored in file /path" vs "stored in OS keychain") and emits the right re-auth guidance per source.logoutwith an env token doesn't pretend to delete the env var — it tells the user to unset it.auth revoke --currentwith an env token treats 401 as idempotent success, but fails loud (non-zero, no "Revoked" claim) on any other server error — env tokens have no local copy to fall back on, so a 5xx must not be silently swallowed.Fixes #1036.
Validation
mise run check(fmt + lint + unit + integration + canary E2E) on every commitentire loginwithENTIRE_SECRETS_PATHset writes the token to a0600JSON file;auth statusreads it back source-tagged. The original "failed to unlock correct collection" error is captured verbatim inTestRunLogin_SaveFailureIncludesHeadlessHint.ENTIRE_AUTH_TOKEN=fake entire auth statustriggers a source-aware "unset ENTIRE_AUTH_TOKEN to re-authenticate" message instead of telling a CI runner to "run entire login".Reviews addressed
auth.go): idempotentrevoke --currentfor env tokens —423551486(with one re-iteration after Codex adversarial review).423551486.deab46b2e.--no-keyringremoved fail-closed login →b203b2308.19b26afc5, then reverted (d63ed2685) for scope. The race requires two concurrent CLI invocations sharing one credentials file, which is exotic in practice; deferred to a follow-up if observed. Atomic temp+rename writes still guarantee readers never see torn content.Notes
ENTIRE_CHECKPOINT_TOKENremains separate and only applies to checkpoint Git fetch and push operations.