Skip to content

fix(web-server): close OAuth token TOCTOU by writing 0o600 atomically - #58006

Closed
Que0x wants to merge 1 commit into
NousResearch:mainfrom
Que0x:fix/web-server-oauth-atomic-write
Closed

fix(web-server): close OAuth token TOCTOU by writing 0o600 atomically#58006
Que0x wants to merge 1 commit into
NousResearch:mainfrom
Que0x:fix/web-server-oauth-atomic-write

Conversation

@Que0x

@Que0x Que0x commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

_save_anthropic_oauth_creds persists the Anthropic OAuth token file with
os.replace(tmp, path) followed by a post-hoc chmod(0o600):

os.replace(tmp_path, _HERMES_OAUTH_FILE)
_HERMES_OAUTH_FILE.chmod(stat.S_IRUSR | stat.S_IWUSR)

Between the rename and the chmod the token file exists at the default umask
(0o644 on most hosts), so for that window another local user can read the
access/refresh tokens.

Write via utils.atomic_json_write(_HERMES_OAUTH_FILE, payload, indent=2, mode=0o600), which creates the temp with mode 0o600 before any content is
written (mkstemp + fchmod), fsyncs, atomically replaces, preserves the
existing file's owner, and cleans up its temp on failure. This is the same
atomic_json_write(mode=0o600) pattern already used elsewhere in this module
for the credential-pool write, and mirrors the owner preservation from #56644.

Changes

  • hermes_cli/web_server.py — persist OAuth creds via
    atomic_json_write(mode=0o600).
  • tests/hermes_cli/test_web_server_oauth_write.py — update the atomicity test
    to the new mechanism and assert the write uses mode=0o600.

Tests

pytest tests/hermes_cli/test_web_server_oauth_write.py -q

The write now goes through atomic_json_write(mode=0o600) (mutation-verified —
reverting drops the call and the assertion fails), and a simulated replace
failure leaves no partial or temp file behind.

`_save_anthropic_oauth_creds` wrote the Anthropic OAuth token file with
`os.replace(tmp, path)` followed by a post-hoc `chmod(0o600)`. Between the
rename and the chmod the token file existed at the default umask (0o644 on most
hosts) — a window in which another local user could read the access/refresh
tokens.

Write via `utils.atomic_json_write(..., mode=0o600)`, which creates the temp
with mode 0o600 *before* any content is written, fsyncs, atomically replaces,
preserves the existing file's owner, and cleans up its temp on failure. This
matches the `atomic_json_write(mode=0o600)` call already used elsewhere in this
module for the credential-pool write, and NousResearch#56644's owner preservation.

Tests updated for the new mechanism, plus a check that the write goes through
`atomic_json_write(mode=0o600)` (mutation-verified).
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists labels Jul 4, 2026
@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

Security evidence: The affected boundary is Anthropic OAuth access/refresh token persistence from the dashboard OAuth callback into _HERMES_OAUTH_FILE; current-main probing showed the old os.replace() path produced a 0o644 credentials file under umask 0o022 before the later chmod, while PR-head probing showed atomic_json_write(..., mode=0o600) leaves the target at 0o600 immediately after the normal same-directory replace path, and CodeRabbit completed with no findings, the PR is limited to routing this credential write through the helper plus focused tests, current GitHub main merge-tree validation succeeded, git diff --check passed, tests/hermes_cli/test_web_server_oauth_write.py passed (3 passed), and atomic write helper coverage passed (33 passed).

Signed: GPT-5.5-xhigh in Codex

@teknium1

teknium1 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #60236 (#60236) — your commit was cherry-picked onto current main with your authorship preserved in git log. The only adjustment was reapplying it onto the per-profile _get_hermes_oauth_file() path that landed in #59339 after you opened this. Solid catch on the chmod-after-replace window, and thanks for matching the existing atomic_json_write(mode=0o600) pattern.

@teknium1 teknium1 closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants