From da28d5d113956dcf803d5cff552a120740a96a59 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Wed, 3 Jun 2026 02:20:29 -0700 Subject: [PATCH] fix(security): gate cp/mv/install into ~/.ssh, credential, and shell-rc files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tools/approval.py already denies tee/redirection writes to every _SENSITIVE_WRITE_TARGET (~/.ssh/*, ~/.netrc/.pgpass/.npmrc/.pypirc, shell rc files, ~/.hermes/config.yaml/.env) via the DANGEROUS_PATTERNS tee/`>` rules, but cp/mv/install were only paired for _SYSTEM_CONFIG_PATH (/etc) and the project-relative env/config target. So `cp evil ~/.ssh/authorized_keys` (SSH-key implant / persistence), `cp creds ~/.netrc`, and `cp evil ~/.bashrc` (login-time command injection) auto-approved while the equivalent tee/`>` forms were denied — an unpaired write deny is theater (same rationale as #14639 / commit 4e9d886d, which paired the terminal side for ~/.hermes/config.yaml writes but did not touch these cp/mv/install verbs on the broader sensitive set). Add one (cp|mv|install) DANGEROUS_PATTERNS entry reusing the existing _SENSITIVE_WRITE_TARGET fragment, anchored via _COMMAND_TAIL so it fires on the destination (last arg) only: reading OUT of a sensitive path (`cp ~/.ssh/config /tmp/x`) stays auto-approved. Description differs from the system-config cp entry so the two keep distinct approval keys (no silent cross-approval). Additive — does not subsume the /etc or project-config rules. Adds TestSensitiveCopyMovePattern: 5 positive cases (ssh authorized_keys, ssh private key via mv, netrc via install, bashrc, ~/.hermes/config.yaml) + 2 negative guards (copy FROM ssh, unrelated copy). The ssh/netrc/bashrc positives fail on main and pass on this branch; the negatives stay green both ways. --- tests/tools/test_approval.py | 37 ++++++++++++++++++++++++++++++++++++ tools/approval.py | 14 ++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index b7598380708..49d13b0cd60 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -633,6 +633,43 @@ class TestProjectSensitiveCopyPattern: assert desc is None +class TestSensitiveCopyMovePattern: + """cp/mv/install OVERWRITING ~/.ssh/*, credential files (~/.netrc etc.), + shell rc files, or ~/.hermes/config.yaml/.env must require approval — the + tee/redirection forms were already gated (#14639 family / commit 4e9d886d), + but cp/mv/install on these targets was an unpaired half-door (key implant / + shell-rc command injection slipped through auto-approve).""" + + def test_cp_to_ssh_authorized_keys(self): + dangerous, key, desc = detect_dangerous_command("cp /tmp/evil ~/.ssh/authorized_keys") + assert dangerous is True + assert key is not None + + def test_mv_to_ssh_private_key(self): + dangerous, key, desc = detect_dangerous_command("mv /tmp/k ~/.ssh/id_rsa") + assert dangerous is True + + def test_install_to_netrc(self): + dangerous, key, desc = detect_dangerous_command("install -m600 /tmp/c ~/.netrc") + assert dangerous is True + + def test_cp_to_bashrc(self): + dangerous, key, desc = detect_dangerous_command("cp /tmp/e ~/.bashrc") + assert dangerous is True + + def test_cp_to_hermes_config(self): + dangerous, key, desc = detect_dangerous_command("cp /tmp/evil.yaml ~/.hermes/config.yaml") + assert dangerous is True + + def test_cp_from_ssh_is_safe(self): + dangerous, key, desc = detect_dangerous_command("cp ~/.ssh/config /tmp/x") + assert dangerous is False + + def test_cp_unrelated_files_safe(self): + dangerous, key, desc = detect_dangerous_command("cp a.txt b.txt") + assert dangerous is False + + class TestProjectSensitiveTeePattern: def test_tee_to_local_dotenv_requires_approval(self): dangerous, key, desc = detect_dangerous_command("printenv | tee .env.local") diff --git a/tools/approval.py b/tools/approval.py index 3baf1fd10d5..9cc7adcc109 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -441,6 +441,20 @@ DANGEROUS_PATTERNS = [ # /private/etc/ mirror). (rf'\b(cp|mv|install)\b.*\s{_SYSTEM_CONFIG_PATH}', "copy/move file into system config path"), (rf'\b(cp|mv|install)\b.*\s["\']?{_PROJECT_SENSITIVE_WRITE_TARGET}["\']?{_COMMAND_TAIL}', "overwrite project env/config file"), + # cp/mv/install OVERWRITING a sensitive credential/SSH/shell-rc/Hermes file. + # The tee/redirection patterns above already gate _SENSITIVE_WRITE_TARGET + # (~/.ssh/*, ~/.netrc/.pgpass/.npmrc/.pypirc, shell rc files, + # ~/.hermes/config.yaml/.env), but cp/mv/install was only paired for /etc and + # project-relative env/config — so `cp evil ~/.ssh/authorized_keys` (key + # implant), `cp creds ~/.netrc`, and `cp evil ~/.bashrc` (login-time command + # injection) slipped through with auto-approve. Same unpaired-door rationale + # as #14639 / the sed-tee-redirect pairing on these targets. + # Anchor the sensitive target to the command tail so this fires on the + # DESTINATION (last arg) only — `cp evil ~/.ssh/authorized_keys` is gated, + # but reading OUT of a sensitive path (`cp ~/.ssh/config /tmp/x`) stays safe. + # The trailing `[^\s"\']*` consumes the rest of the destination filename + # (e.g. `authorized_keys` after the `~/.ssh/` fragment). + (rf'\b(cp|mv|install)\b.*\s["\']?{_SENSITIVE_WRITE_TARGET}[^\s"\']*["\']?{_COMMAND_TAIL}', "copy/move file into sensitive credential/SSH/shell-rc path"), (rf'\bsed\s+-[^\s]*i.*\s{_SYSTEM_CONFIG_PATH}', "in-place edit of system config"), (rf'\bsed\s+--in-place\b.*\s{_SYSTEM_CONFIG_PATH}', "in-place edit of system config (long flag)"), # In-place edit of a Hermes-managed security file (~/.hermes/config.yaml or