fix(server,core): clarify Claude auth posture on per-user installs (#1983 hardening)#2002
Merged
Conversation
…1983 hardening) Follow-up hardening to the #1983/#1984 investigation. No behavior change for solo installs; removes the misleading global-auth signal on per-user installs and makes the not-logged-in case actionable. - Extract the boot auth-posture decision into pure, testable helpers (server/src/boot/claude-auth-posture.ts): `shouldDefaultClaudeGlobalAuth` and `hasClaudeBootAuthPosture`. The `CLAUDE_USE_GLOBAL_AUTH=true` boot auto-default is now SKIPPED when per-user keys are enabled (TOKEN_ENCRYPTION_KEY) — those installs deliver Claude auth per-request, so the sentinel was misleading (it made the Claude provider log `using_global_auth` on installs that authenticated fine, which derailed the #1983 triage). The boot credential check now treats per-user-keys as a valid posture so a per-user-only install (no shared Claude key) doesn't exit(1). - Fix the stale auth comment in claude/provider.ts: `buildSubprocessEnv` has not filtered env tokens since #1067; document that per-request `requestOptions.env` merges last and that CLAUDE_USE_GLOBAL_AUTH is an Archon-only boot sentinel the CLI ignores. - Make the "Not logged in" chat error actionable: carry the SDK error detail (not just the subtype) into classifyAndFormatError, and add a not-logged-in pattern that names the connect surfaces (Settings → Agents / `claude /login` / CLAUDE_API_KEY) instead of leaking the raw CLI string. Tests: claude-auth-posture.test.ts (env matrix), error-formatter not-logged-in cases. Solo path unchanged (no creds → still defaults to global auth). Closes #1983.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughMoves Claude boot auth checks into a dedicated helper, updates server boot wiring and tests, adds a Claude provider auth comment update, and introduces not-logged-in error classification using richer orchestrator error context. ChangesClaude Auth Posture and Error Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/server/src/boot/claude-auth-posture.ts`:
- Around line 35-40: The hasClaudeBootAuthPosture function uses truthiness to
check the CLAUDE_USE_GLOBAL_AUTH environment variable, which incorrectly treats
the string 'false' as truthy and counts it as valid auth posture. Replace the
CLAUDE_USE_GLOBAL_AUTH condition in the Boolean check with an explicit equality
comparison (=== 'true') to ensure only the string 'true' is treated as a valid
Claude authentication posture.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad264f5a-dc5c-402c-82ac-ac5629de200a
📒 Files selected for processing (8)
packages/core/src/orchestrator/orchestrator-agent.tspackages/core/src/utils/error-formatter.test.tspackages/core/src/utils/error-formatter.tspackages/providers/src/claude/provider.tspackages/server/package.jsonpackages/server/src/boot/claude-auth-posture.test.tspackages/server/src/boot/claude-auth-posture.tspackages/server/src/index.ts
…posture hasClaudeBootAuthPosture used plain truthiness on the sentinel, so an explicit CLAUDE_USE_GLOBAL_AUTH=false (which setup.ts writes) wrongly counted as a valid Claude auth posture and suppressed the no-credentials boot warn/exit. Parse it enabled-only (=== 'true'), matching the codebase convention (ARCHON_DOCKER, ARCHON_AUTH_OPEN_SIGNUP). Adds regression tests for the 'false' and arbitrary-value cases. Addresses CodeRabbit review on claude-auth-posture.ts.
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
1 participant
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.
Summary
CLAUDE_USE_GLOBAL_AUTH=trueboot auto-default and a stale "filters env tokens" comment made per-user installs look like they were on global auth when they were actually authenticating fine per-request — which derailed the Per-user Claude subscription unusable in chat: CLAUDE_USE_GLOBAL_AUTH boot default filters delivered tokens #1983 triage. The not-logged-in chat error was also generic.UX Journey
Before
After
Architecture Diagram
After
Connection inventory:
Label Snapshot
risk: lowsize: Sserver,core,testsserver:boot,core:orchestrator,core:utils,providers:claude(comment only)Change Metadata
bug(hardening / clarity)multi(server + core)Linked Issue
Closes #1983is in the commit for traceability; it's a no-op since the issue is closed.Validation Evidence (required)
bun run validate # all guards + type-check + lint + format:check + batched testsbun run validategreen. New/updated tests:packages/server/src/boot/claude-auth-posture.test.ts— env matrix for both helpers (solo→default, per-user→skip, explicit creds, operator-set, empty-string).packages/core/src/utils/error-formatter.test.ts— not-logged-in cases (names connect surfaces, doesn't leak raw string).@archon/serverpackage.json batch.Security Impact (required)
NoNoNo— only changes whether a non-secret boot sentinel is set; no credential read/delivery path touched.NoCompatibility / Migration
Yes— solo installs behave identically (verified by test: no creds → default true; explicit creds → false; operator-set → never overridden).NoNoHuman Verification (required)
bun run validate; the Per-user Claude subscription unusable in chat: CLAUDE_USE_GLOBAL_AUTH boot default filters delivered tokens #1983/Subscriptions can't power Pi in direct chat: env-only delivery has no auth.json path and the env fallback is Claude-Code-shaped #1984 behavior was live-verified on the VPS post-fix(credentials,pi): deliver Claude subscriptions to Pi in direct chat (#1984) #2000 deploy (native Claudeclaude.rate_limit_event {allowed}, Pi/anthropicpi.prompt_completed).hasClaudeBootAuthPostureincludesTOKEN_ENCRYPTION_KEY).Side Effects / Blast Radius (required)
TOKEN_ENCRYPTION_KEY) and surface the actionable not-logged-in message at chat time — intended.claude_credentials_missingwarn still fires when nothing resolves;ai_result_errorlog on failed turns.Rollback Plan (required)
exit(1)at boot with "no_ai_credentials" on an install that should be valid (would indicatehasClaudeBootAuthPostureregressed).Risks and Mitigations
CLAUDE_USE_GLOBAL_AUTH=trueauto-default AND hadTOKEN_ENCRYPTION_KEYset but expected global auth.CLAUDE_USE_GLOBAL_AUTH=trueexplicitly (the helper never overrides an explicit value). Per-user installs are precisely the ones that should NOT use a shared global login.Summary by CodeRabbit