fix(security): salvage #30553 + #11004 OAuth file-safety hardening - #31747
Merged
Conversation
…ees as other auth paths The web dashboard's Anthropic OAuth helper wrote the credential file straight to its final destination and relied on the process umask for permissions. That left the dashboard-specific path weaker than the existing auth writers, which already use owner-only permissions and safer write semantics. This change keeps the scope narrow: make the dashboard helper write via a temp file + replace, chmod the final file to owner-only, and add a focused regression test for both permission handling and atomic-write behavior. Constraint: Must preserve the existing dashboard OAuth flow and credential-pool side effects Rejected: Broader auth-storage refactor | unnecessary scope for a single verified inconsistency Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep dashboard credential writes aligned with existing auth storage semantics; do not reintroduce direct write_text() here without matching chmod/atomic behavior Tested: pytest -o addopts='' tests/hermes_cli/test_web_server_oauth_write.py tests/hermes_cli/test_web_server.py -q (78 passed) Not-tested: Cross-platform permission semantics on Windows-managed filesystems
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
1 |
First entries
tests/hermes_cli/test_web_server_oauth_write.py:3: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
✅ Fixed issues: none
Unchanged: 4850 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
8 tasks
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.
Salvage of two stale OAuth-related PRs that conflicted with the just-merged batch (#27217, #30972, #21152).
Commits (rebased onto current main, contributor authorship preserved)
@kronexoi — fix(security): restrict write access to Anthropic OAuth credential store (fix(security): restrict write access to Anthropic OAuth credential store #30553)
Adds
{HERMES_HOME, <root>}/.anthropic_oauth.jsonto the write-deny list.Closes the read-vs-write asymmetry — agent could not READ the file but could overwrite it (silent OAuth kick).
@shaun0927 — Protect dashboard OAuth credentials with file-safety guarantees (fix(web): write dashboard Anthropic OAuth creds atomically with 0600 perms #11004)
Replaces
write_text+ post-replacechmodin_save_anthropic_oauth_credswith atomic create-at-0o600 (mkstemp + fsync + os.replace). Mirrors the fix shape we landed for google_oauth (fix(google_oauth): close TOCTOU window when saving credentials #19673), mcp_oauth (fix(security): close TOCTOU window when saving MCP OAuth credentials #21148), Claude Code creds (fix(security): close TOCTOU window when saving Claude Code OAuth credentials #21152), and webhook subscriptions (fix(state): restrict sensitive store file permissions (salvage #30917) #31469).Test plan
Co-authored-by: kronexoi kronexoi13@gmail.com
Co-authored-by: shaun0927 70629228+shaun0927@users.noreply.github.com
Closes #30553
Closes #11004