Protect dashboard OAuth credentials with the same file-safety guarantees 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
This commit is contained in:
JunghwanNA 2026-04-16 21:10:05 +09:00 committed by Teknium
parent 55987818b6
commit 243ebc7a61
2 changed files with 73 additions and 1 deletions

View file

@ -16,6 +16,7 @@ import json
import logging
import os
import secrets
import stat
import subprocess
import sys
import threading
@ -1686,7 +1687,25 @@ def _save_anthropic_oauth_creds(access_token: str, refresh_token: str, expires_a
"expiresAt": expires_at_ms,
}
_HERMES_OAUTH_FILE.parent.mkdir(parents=True, exist_ok=True)
_HERMES_OAUTH_FILE.write_text(json.dumps(payload, indent=2), encoding="utf-8")
tmp_path = _HERMES_OAUTH_FILE.with_name(
f"{_HERMES_OAUTH_FILE.name}.tmp.{os.getpid()}.{secrets.token_hex(8)}"
)
try:
with tmp_path.open("w", encoding="utf-8") as handle:
handle.write(json.dumps(payload, indent=2))
handle.flush()
os.fsync(handle.fileno())
os.replace(tmp_path, _HERMES_OAUTH_FILE)
try:
_HERMES_OAUTH_FILE.chmod(stat.S_IRUSR | stat.S_IWUSR)
except OSError:
pass
finally:
try:
if tmp_path.exists():
tmp_path.unlink()
except OSError:
pass
# Best-effort credential-pool insert. Failure here doesn't invalidate
# the file write — pool registration only matters for the rotation
# strategy, not for runtime credential resolution.

View file

@ -0,0 +1,53 @@
import os
import pytest
from hermes_cli.web_server import _save_anthropic_oauth_creds
class _DummyPool:
def entries(self):
return []
def remove_entry(self, _id):
return None
def add_entry(self, _entry):
return None
@pytest.fixture
def oauth_file(monkeypatch, tmp_path):
target = tmp_path / '.anthropic_oauth.json'
monkeypatch.setattr('agent.anthropic_adapter._HERMES_OAUTH_FILE', target)
monkeypatch.setattr('agent.credential_pool.load_pool', lambda _provider: _DummyPool())
return target
def test_dashboard_oauth_write_uses_owner_only_permissions(oauth_file):
old_umask = os.umask(0o022)
try:
_save_anthropic_oauth_creds('access-token', 'refresh-token', 123456)
finally:
os.umask(old_umask)
assert oauth_file.exists()
mode = oauth_file.stat().st_mode & 0o777
assert mode == 0o600
def test_dashboard_oauth_write_uses_atomic_replace_and_cleans_temp_files(oauth_file, monkeypatch):
replace_calls = []
def flaky_replace(src, dst):
replace_calls.append((src, dst))
raise OSError('simulated replace failure')
monkeypatch.setattr('hermes_cli.web_server.os.replace', flaky_replace)
with pytest.raises(OSError, match='simulated replace failure'):
_save_anthropic_oauth_creds('access-token', 'refresh-token', 123456)
assert replace_calls, 'helper should attempt atomic os.replace()'
assert not oauth_file.exists()
assert not list(oauth_file.parent.glob(f'{oauth_file.name}.tmp*'))