mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-01 07:01:41 +00:00
fix(security): close TOCTOU window when saving Claude Code OAuth credentials (#21152)
_write_claude_code_credentials wrote ~/.claude/.credentials.json via Path.write_text + replace + post-write chmod(0o600). Both the temp file and the destination briefly inherited the process umask (commonly 0o644 = world-readable) between create/replace and chmod, exposing the OAuth access/refresh tokens to other local users on multi-user hosts. Use os.open with O_WRONLY|O_CREAT|O_EXCL and an explicit S_IRUSR|S_IWUSR mode so the temp file is created atomically at 0o600. After os.replace, the destination inherits the temp's mode, so the post-write chmod is no longer needed. The temp name also gains a per-process random suffix to avoid collisions between concurrent writers and stale leftovers from a crashed prior write. Parent dir (~/.claude/) is owned by Claude Code itself and shared with its native auth, so we deliberately don't tighten its mode here (unlike the mcp_oauth fix which owns its own subtree under HERMES_HOME). Mirrors the fix shipped for agent/google_oauth.py in #19673 and the parallel fix for tools/mcp_oauth.py in #21148. Adds a regression test in TestWriteClaudeCodeCredentials asserting the resulting file mode is 0o600 (skipped on Windows where POSIX mode bits aren't enforced).
This commit is contained in:
parent
bba76f3dcd
commit
223a3971c0
2 changed files with 49 additions and 5 deletions
|
|
@ -15,6 +15,8 @@ import json
|
|||
import logging
|
||||
import os
|
||||
import platform
|
||||
import secrets
|
||||
import stat
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlparse
|
||||
|
|
@ -1040,11 +1042,34 @@ def _write_claude_code_credentials(
|
|||
existing["claudeAiOauth"] = oauth_data
|
||||
|
||||
cred_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
_tmp_cred = cred_path.with_suffix(".tmp")
|
||||
_tmp_cred.write_text(json.dumps(existing, indent=2), encoding="utf-8")
|
||||
_tmp_cred.replace(cred_path)
|
||||
# Restrict permissions (credentials file)
|
||||
cred_path.chmod(0o600)
|
||||
# Per-process random suffix avoids collisions between concurrent
|
||||
# writers and stale leftovers from a prior crashed write.
|
||||
_tmp_cred = cred_path.with_suffix(f".tmp.{os.getpid()}.{secrets.token_hex(4)}")
|
||||
try:
|
||||
# Create the temp file atomically at 0o600. The previous
|
||||
# write_text + post-replace chmod opened a TOCTOU window where
|
||||
# both the temp file and the destination briefly inherited the
|
||||
# process umask (commonly 0o644 = world-readable), exposing
|
||||
# Claude Code OAuth tokens to other local users between create
|
||||
# and chmod. Mirrors agent/google_oauth.py (#19673) and
|
||||
# tools/mcp_oauth.py (#21148). Parent dir (~/.claude/) is
|
||||
# owned by Claude Code itself, so we leave its mode alone.
|
||||
fd = os.open(
|
||||
str(_tmp_cred),
|
||||
os.O_WRONLY | os.O_CREAT | os.O_EXCL,
|
||||
stat.S_IRUSR | stat.S_IWUSR,
|
||||
)
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as fh:
|
||||
json.dump(existing, fh, indent=2)
|
||||
fh.flush()
|
||||
os.fsync(fh.fileno())
|
||||
os.replace(_tmp_cred, cred_path)
|
||||
except OSError:
|
||||
try:
|
||||
_tmp_cred.unlink(missing_ok=True)
|
||||
except OSError:
|
||||
pass
|
||||
raise
|
||||
except (OSError, IOError) as e:
|
||||
logger.debug("Failed to write refreshed credentials: %s", e)
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
"""Tests for agent/anthropic_adapter.py — Anthropic Messages API adapter."""
|
||||
|
||||
import json
|
||||
import sys
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
|
@ -420,6 +421,24 @@ class TestWriteClaudeCodeCredentials:
|
|||
assert data["otherField"] == "keep-me"
|
||||
assert data["claudeAiOauth"]["accessToken"] == "new-tok"
|
||||
|
||||
@pytest.mark.skipif(sys.platform.startswith("win"), reason="POSIX mode bits not enforced on Windows")
|
||||
def test_credentials_file_created_with_0o600(self, tmp_path, monkeypatch):
|
||||
"""Refreshed Claude Code credentials must land on disk at 0o600.
|
||||
|
||||
Regression for the TOCTOU race where ``write_text`` + ``replace``
|
||||
+ post-write ``chmod`` left both the temp file and the destination
|
||||
briefly readable at the process umask (commonly 0o644). Mirrors
|
||||
the fix shipped in #19673 (google_oauth) and #21148 (mcp_oauth).
|
||||
"""
|
||||
import stat as _stat
|
||||
monkeypatch.setattr("agent.anthropic_adapter.Path.home", lambda: tmp_path)
|
||||
_write_claude_code_credentials("tok", "ref", 12345)
|
||||
|
||||
cred_file = tmp_path / ".claude" / ".credentials.json"
|
||||
assert cred_file.exists()
|
||||
mode = _stat.S_IMODE(cred_file.stat().st_mode)
|
||||
assert mode == 0o600, f"creds file mode {oct(mode)} != 0o600 — TOCTOU race regressed"
|
||||
|
||||
|
||||
class TestResolveWithRefresh:
|
||||
def test_auto_refresh_on_expired_creds(self, monkeypatch, tmp_path):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue