Skip to content

fix(safety): casefold credential path guards - #52045

Open
lin-hongkuan wants to merge 1 commit into
NousResearch:mainfrom
lin-hongkuan:codex/fix-51474-credential-path-casefold
Open

fix(safety): casefold credential path guards#52045
lin-hongkuan wants to merge 1 commit into
NousResearch:mainfrom
lin-hongkuan:codex/fix-51474-credential-path-casefold

Conversation

@lin-hongkuan

Copy link
Copy Markdown
Contributor

Summary

  • compare credential deny paths case-insensitively on Windows and macOS while keeping Linux case-sensitive
  • apply the same comparison to write guards, read guards, and media-delivery deny checks
  • add regression coverage for .SSH, .AWS, AUTH.JSON, .ENV, and media delivery of case-variant SSH keys

Fixes #51474.

Tests

  • python -m pytest tests/tools/test_write_deny.py tests/agent/test_file_safety_credentials.py tests/agent/test_file_safety.py tests/gateway/test_platform_base.py::TestMediaDeliveryDefaultMode tests/gateway/test_platform_base.py::TestMediaDeliveryPathValidation -q
  • python -m py_compile agent/file_safety.py gateway/platforms/base.py
  • python scripts/check-windows-footguns.py --all

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data platform/windows Native Windows-specific behavior or breakage duplicate This issue or pull request already exists labels Jun 24, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #51475 — same fix for #51474 (casefold credential deny-path comparisons on case-insensitive filesystems), touching the same files (agent/file_safety.py, gateway/platforms/base.py) with the same mechanism. #51475 (the issue+PR pair fix) arrived first and is the canonical version. Linking rather than closing.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for covering the write, read, and media-delivery deny paths together. The underlying issue is still present on current main: agent/file_safety.py:103-106 uses direct equality/startswith, and gateway/platforms/base.py:1229-1234 uses case-sensitive path containment.

Problems

Suggested changes

  • Salvage one canonical patch onto current HEAD, preserving the multiple-safe-root behavior and deterministic platform-branch regression tests.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
bbasketballer75 added a commit to bbasketballer75/hermes-agent that referenced this pull request Aug 13, 2026
_MEDIA_DELIVERY_DENIED_PREFIXES is POSIX-only, and the entries do not degrade
gracefully. Path("/etc").resolve() becomes C:\etc on the current drive, which
does not exist -- so on Windows every entry in that tuple is inert and the
default-mode denylist is effectively empty.

Measured on Windows against unmodified main:

    /etc     -> C:\etc      exists=False
    /sys     -> C:\sys      exists=False
    /root    -> C:\root     exists=False
    ... all nine resolve to nonexistent paths

    C:\Windows      covered=False
    C:\ProgramData  covered=False

Consequence, verified end-to-end through the guard itself:

    validate_media_delivery_path(r"C:\Windows\win.ini")
      before -> <path>   (deliverable as a gateway attachment)
      after  -> None     (refused)

Strict mode was unaffected -- the ~/.ssh denial works, because those entries
are built from the live $HOME rather than hard-coded POSIX roots. This is the
default (non-strict) path, which is what most operators run.

Adds two Windows-specific lists:

  - system roots resolved from %SystemRoot% and %ProgramData%, read from the
    environment rather than hard-coded, because Windows need not live on C:
    and a roaming profile need not sit under C:\Users.

  - the credential stores under the user profile that have no POSIX
    counterpart: Microsoft\Credentials (both roaming and local),
    Microsoft\Protect (DPAPI master keys), Microsoft\Crypto, and
    Local\Microsoft\Vault. The POSIX dotfile equivalents (.aws, .ssh, .azure,
    .gcloud) use the same names on Windows and are already covered by
    _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS.

AppData is deliberately NOT denied wholesale. %LOCALAPPDATA%\Temp is a normal
home for generated artifacts, so a blanket rule would break legitimate media
delivery. A test asserts Temp, Pictures and Downloads stay deliverable.

Verification on Windows:
  - 9 new tests; 6 of them fail when the production change is reverted, so the
    coverage pins real behaviour rather than passing vacuously. The 3 that pass
    either way are the must-stay-deliverable assertions.
  - tests/gateway/test_platform_base.py: 2 failed before, 2 failed after --
    set-diff of failure names is empty, nothing newly broken. Those 2 are
    unrelated pre-existing failures.
  - ruff clean.

Complementary to (not overlapping) the case-folding work in NousResearch#52045: that PR
rewrites the comparison helpers, this one fixes what is in the list to compare
against. Different hunks in the same file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows 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.

Bug: credential-path deny heuristics are case-sensitive — bypassed on case-insensitive filesystems (macOS/APFS, Windows)

3 participants