pdurlej/platform

Fork 0

test(agent-access): cover signal cleanup path #90

Merged

pdurlej merged 1 commit from codex/issues/79-signal-cleanup-test into main

2026-05-05 23:53:18 +02:00

codex commented

2026-05-05 23:43:35 +02:00

Collaborator

Canary status: approve_merge — 6/6 reviewers OK on cbf04d7720476a760c061456857c6d5d3d1f182d; weak-signal time import finding is false-positive (time is imported at line 11).

Canary Context Pack

Product story

Security-sensitive agent access work should not rely on prose claims when a small regression test can prove the behavior. PR #84 shipped the cleanup hardening; this follow-up locks the signal cleanup path that canary asked to see directly.

What changed

Adds one focused test that starts the Codex OpenClaw ssh-agent wrapper, blocks it during the post-start fingerprint check, sends SIGTERM and SIGINT in separate parametrized runs, and verifies the wrapper kills the dedicated ssh-agent and does not leak private-key material to output or runtime files.

Why it changed

PR #84 canary produced a product dissent: the PR claimed SIGINT/SIGTERM use the same cleanup path, but the direct evidence was missing. #84 was merged before this extra test landed on main, so this PR carries only the missing regression test.

Files touched

control-plane/platformctl/tests/test_agent_access_ssh_agent.py

Relevant context

Follow-up to #84.
Refs #79 and #83.
Security-sensitive lane from #82 applies: smallest PR, explicit tests, canary before merge.

Runtime evidence

PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py -> 10 passed
PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests -> 166 passed
git diff --check origin/main...HEAD -> pass
Diff secret-pattern scan -> pass

Known constraints

The test uses disposable fake OpenSSH tools only. It does not use a real OpenClaw private key, Forgejo PAT, Infisical secret value, or live VPS SSH session.

Explicit out-of-scope

No wrapper behavior change.
No Infisical, Forgejo PAT, or CI secret changes.
No generic agent-access catalog/list/prune work.

Requested decision

Approve merge if canary confirms this test closes the direct-evidence gap without introducing new security/process risk.

Merge blockers

Test flake or failure.
Canary finding that the test does not exercise the intended signal cleanup path.
Any secret leakage concern in test output, argv, env handling, or fixtures.

Spec sources read

AGENTS.md — security-sensitive lane, canary, identity and PR body requirements.
control-plane/platformctl/tests/test_agent_access_ssh_agent.py — test harness and fake OpenSSH tool pattern.
scripts/agent-access/codex-openclaw-ssh-agent — behavior under test from #84 continuation.
PR #84 canary output — direct-evidence gap that this PR addresses.

Refs #79
Refs #83
Follow-up to #84

Canary status: approve_merge — 6/6 reviewers OK on `cbf04d7720476a760c061456857c6d5d3d1f182d`; weak-signal `time` import finding is false-positive (`time` is imported at line 11). ## Canary Context Pack ### Product story Security-sensitive agent access work should not rely on prose claims when a small regression test can prove the behavior. PR #84 shipped the cleanup hardening; this follow-up locks the signal cleanup path that canary asked to see directly. ### What changed Adds one focused test that starts the Codex OpenClaw ssh-agent wrapper, blocks it during the post-start fingerprint check, sends SIGTERM and SIGINT in separate parametrized runs, and verifies the wrapper kills the dedicated ssh-agent and does not leak private-key material to output or runtime files. ### Why it changed PR #84 canary produced a product dissent: the PR claimed SIGINT/SIGTERM use the same cleanup path, but the direct evidence was missing. #84 was merged before this extra test landed on main, so this PR carries only the missing regression test. ### Files touched - `control-plane/platformctl/tests/test_agent_access_ssh_agent.py` ### Relevant context - Follow-up to #84. - Refs #79 and #83. - Security-sensitive lane from #82 applies: smallest PR, explicit tests, canary before merge. ### Runtime evidence - `PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py` -> `10 passed` - `PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests` -> `166 passed` - `git diff --check origin/main...HEAD` -> pass - Diff secret-pattern scan -> pass ### Known constraints The test uses disposable fake OpenSSH tools only. It does not use a real OpenClaw private key, Forgejo PAT, Infisical secret value, or live VPS SSH session. ### Explicit out-of-scope - No wrapper behavior change. - No Infisical, Forgejo PAT, or CI secret changes. - No generic agent-access catalog/list/prune work. ### Requested decision Approve merge if canary confirms this test closes the direct-evidence gap without introducing new security/process risk. ### Merge blockers - Test flake or failure. - Canary finding that the test does not exercise the intended signal cleanup path. - Any secret leakage concern in test output, argv, env handling, or fixtures. ## Spec sources read - `AGENTS.md` — security-sensitive lane, canary, identity and PR body requirements. - `control-plane/platformctl/tests/test_agent_access_ssh_agent.py` — test harness and fake OpenSSH tool pattern. - `scripts/agent-access/codex-openclaw-ssh-agent` — behavior under test from #84 continuation. - PR #84 canary output — direct-evidence gap that this PR addresses. Refs #79 Refs #83 Follow-up to #84

codex added 1 commit

2026-05-05 23:43:36 +02:00

test(agent-access): cover signal cleanup path

canary-required / collect-diff (pull_request) Successful in 3s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 15s

Details

python-ci / Python 3.11 (pull_request) Successful in 28s

Details

python-ci / Python 3.12 (pull_request) Successful in 29s

Details

python-ci / Python 3.13 (pull_request) Successful in 29s

Details

canary-required / canary (pull_request) Successful in 12s

Details

48f2dab309

codex added the

class/security-sensitive

label

2026-05-05 23:43:47 +02:00

codex force-pushed codex/issues/79-signal-cleanup-test from 48f2dab309

canary-required / collect-diff (pull_request) Successful in 3s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 15s

Details

python-ci / Python 3.11 (pull_request) Successful in 28s

Details

python-ci / Python 3.12 (pull_request) Successful in 29s

Details

python-ci / Python 3.13 (pull_request) Successful in 29s

Details

canary-required / canary (pull_request) Successful in 12s

Details

to cbf04d7720

canary-required / collect-diff (pull_request) Successful in 3s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 15s

Details

python-ci / Python 3.11 (pull_request) Successful in 28s

Details

python-ci / Python 3.12 (pull_request) Successful in 28s

Details

python-ci / Python 3.13 (pull_request) Successful in 28s

Details

canary-required / canary (pull_request) Successful in 12s

Details

2026-05-05 23:47:22 +02:00

Compare

codex commented

2026-05-05 23:51:00 +02:00

Author

Collaborator

Canary iteration 2: approve_merge on cbf04d7720476a760c061456857c6d5d3d1f182d. 6/6 reviewers OK.

Local evidence before canary: focused test 10 passed, full platformctl/tests 166 passed, git diff --check pass, secret-pattern scan pass.

Weak-signal note: Missing time import is a false positive; time is imported in control-plane/platformctl/tests/test_agent_access_ssh_agent.py line 11 and is used by _wait_for_file. Low timing-margin comments are non-blocking under the approved canary result.

Canary iteration 2: `approve_merge` on `cbf04d7720476a760c061456857c6d5d3d1f182d`. 6/6 reviewers OK. Local evidence before canary: focused test `10 passed`, full `platformctl/tests` `166 passed`, `git diff --check` pass, secret-pattern scan pass. Weak-signal note: `Missing time import` is a false positive; `time` is imported in `control-plane/platformctl/tests/test_agent_access_ssh_agent.py` line 11 and is used by `_wait_for_file`. Low timing-margin comments are non-blocking under the approved canary result.