fix(agent-access): harden ssh-agent startup cleanup #84

Merged

pdurlej merged 5 commits from codex/issues/79-startup-cleanup into main 2026-05-05 23:39:38 +02:00

Conversation 1 Commits 5 Files changed 2 +124 -16

codex commented 2026-05-05 08:46:45 +02:00

Collaborator

Copy link

Canary status: missing — rerunning after signal-mask child fix on 339ff91cefecfa8f2b2451abd1dda9c1e0f826a9 with PR body supplied.

Refresh after #89 / #82 merge

• Merged latest origin/main into codex/issues/79-startup-cleanup without force-push.
• Resolved the SSH-agent TTL test conflict by keeping the mainline stable ttl=5s / sleep=6s timing.
• Local verification: python3 -m pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py -> 8 passed.
• Local verification: python3 -m pytest -q control-plane/platformctl/tests -> 164 passed.
• Local verification: pyfallow analyze --root control-plane --format agent-fix-plan --fail-on error --min-confidence medium -> blocking_count=0.
• Forgejo status on 8624f23: 6/6 checks success (canary-required, pyfallow, Python 3.11/3.12/3.13).

Canary Context Pack

Product story

The Codex -> OpenClaw SSH wrapper should fail closed. If the wrapper starts an ssh-agent and then fails before producing usable session state, it must clean up that agent instead of leaving an access-bearing process behind.

What changed

• Runtime directory creation now uses a single mkdir path and rejects symlink/non-directory collisions.
• If ssh-agent starts but its output cannot be parsed, the parsed PID is terminated best-effort.
• SIGINT/SIGTERM enter the same cleanup path as normal wrapper failures.
• Agent cleanup logic is centralized so post-start failure paths clear the private-key variable and call ssh-agent -k when socket/PID are known.
• Test fake OpenSSH now records ssh-agent -k calls.
• Added regression test for post-start failure after key load / before fingerprint success.
• Real OpenSSH TTL test uses 3s instead of 1s to avoid test flake where the key expires before the first assertion.

Why it changed

This is the first split from superseded PR #80. It keeps the security-sensitive surface narrow: startup failure cleanup only, no --list, no --prune, no session status taxonomy.

Files touched

• scripts/agent-access/codex-openclaw-ssh-agent
• control-plane/platformctl/tests/test_agent_access_ssh_agent.py

Relevant context

• Parent hardening issue #79.
• Child issue #83.
• Superseded combined PR #80.
• Operator ruling: security-sensitive work should use smaller PRs and full canary; more friction is a feature.

Runtime evidence

• PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py -> 8 passed
• PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests -> 164 passed
• git diff --check origin/main...HEAD -> pass
• Secret-pattern diff scan -> no matches

Known constraints

Signal handling still performs cleanup from Python exception flow; this is good enough for this wrapper but not a general async-signal-safe framework.

Explicit out-of-scope

• --list
• --prune
• Session lifecycle inventory/status classification
• Runtime layout docs for new commands
• Infisical scope changes
• OpenClaw runtime changes

Requested decision

Approve if this narrow startup cleanup reduces residual access risk without widening the wrapper behavior.

Merge blockers

• Any private-key leak to stdout/stderr, child env, argv, repo, or runtime files.
• Any cleanup path that can leave a known started test agent alive after tested failure.
• Scope creep into list/prune/session inventory.

Spec sources read

• Issue #83 body: direct child scope.
• Issue #79 context from prior canary findings.
• scripts/agent-access/codex-openclaw-ssh-agent: implementation target.
• control-plane/platformctl/tests/test_agent_access_ssh_agent.py: regression coverage target.
• PR #80 canary findings: reason for split/rewrite.

Closes #83
Refs #79

Canary status: missing — rerunning after signal-mask child fix on `339ff91cefecfa8f2b2451abd1dda9c1e0f826a9` with PR body supplied. ## Refresh after #89 / #82 merge - Merged latest `origin/main` into `codex/issues/79-startup-cleanup` without force-push. - Resolved the SSH-agent TTL test conflict by keeping the mainline stable `ttl=5s` / `sleep=6s` timing. - Local verification: `python3 -m pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py` -> `8 passed`. - Local verification: `python3 -m pytest -q control-plane/platformctl/tests` -> `164 passed`. - Local verification: `pyfallow analyze --root control-plane --format agent-fix-plan --fail-on error --min-confidence medium` -> `blocking_count=0`. - Forgejo status on `8624f23`: 6/6 checks success (`canary-required`, `pyfallow`, Python 3.11/3.12/3.13). ## Canary Context Pack ### Product story The Codex -> OpenClaw SSH wrapper should fail closed. If the wrapper starts an `ssh-agent` and then fails before producing usable session state, it must clean up that agent instead of leaving an access-bearing process behind. ### What changed - Runtime directory creation now uses a single mkdir path and rejects symlink/non-directory collisions. - If `ssh-agent` starts but its output cannot be parsed, the parsed PID is terminated best-effort. - SIGINT/SIGTERM enter the same cleanup path as normal wrapper failures. - Agent cleanup logic is centralized so post-start failure paths clear the private-key variable and call `ssh-agent -k` when socket/PID are known. - Test fake OpenSSH now records `ssh-agent -k` calls. - Added regression test for post-start failure after key load / before fingerprint success. - Real OpenSSH TTL test uses 3s instead of 1s to avoid test flake where the key expires before the first assertion. ### Why it changed This is the first split from superseded PR #80. It keeps the security-sensitive surface narrow: startup failure cleanup only, no `--list`, no `--prune`, no session status taxonomy. ### Files touched - `scripts/agent-access/codex-openclaw-ssh-agent` - `control-plane/platformctl/tests/test_agent_access_ssh_agent.py` ### Relevant context - Parent hardening issue #79. - Child issue #83. - Superseded combined PR #80. - Operator ruling: security-sensitive work should use smaller PRs and full canary; more friction is a feature. ### Runtime evidence - `PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests/test_agent_access_ssh_agent.py` -> `8 passed` - `PYTHONPATH=control-plane pytest -q control-plane/platformctl/tests` -> `164 passed` - `git diff --check origin/main...HEAD` -> pass - Secret-pattern diff scan -> no matches ### Known constraints Signal handling still performs cleanup from Python exception flow; this is good enough for this wrapper but not a general async-signal-safe framework. ### Explicit out-of-scope - `--list` - `--prune` - Session lifecycle inventory/status classification - Runtime layout docs for new commands - Infisical scope changes - OpenClaw runtime changes ### Requested decision Approve if this narrow startup cleanup reduces residual access risk without widening the wrapper behavior. ### Merge blockers - Any private-key leak to stdout/stderr, child env, argv, repo, or runtime files. - Any cleanup path that can leave a known started test agent alive after tested failure. - Scope creep into list/prune/session inventory. ## Spec sources read - Issue #83 body: direct child scope. - Issue #79 context from prior canary findings. - `scripts/agent-access/codex-openclaw-ssh-agent`: implementation target. - `control-plane/platformctl/tests/test_agent_access_ssh_agent.py`: regression coverage target. - PR #80 canary findings: reason for split/rewrite. Closes #83 Refs #79

codex added 1 commit 2026-05-05 08:46:45 +02:00

fix(agent-access): harden ssh-agent startup cleanup c130b7f83a

codex added the

class/security-sensitive

label 2026-05-05 08:48:31 +02:00

codex force-pushed codex/issues/79-startup-cleanup from c130b7f83a to 818876c68c 2026-05-05 08:55:02 +02:00 Compare

codex force-pushed codex/issues/79-startup-cleanup from 818876c68c to a39c3afc45 2026-05-05 09:01:15 +02:00 Compare

codex referenced this pull request 2026-05-05 09:10:12 +02:00

fix(canary): include PR body in reviewer context #85

codex commented 2026-05-05 09:10:29 +02:00

Author

Collaborator

Copy link

Canary terminal update for PR #84

• Iter 1: defer; mitigated signal/PID capture weak signal.
• Iter 2: defer; mitigated SIGINT/SIGTERM during ssh-agent startup with signal masking.
• Iter 3: defer; tech reviewers are 3/3 OK. Remaining high dissent is PR description: none, but the PR body exists via Forgejo API.

Terminal action: defer_to_issue.

Tracked follow-up: #85 (fix(canary): include PR body in reviewer context). I am not running a 4th canary because the cap is reached.

Canary terminal update for PR #84 - Iter 1: `defer`; mitigated signal/PID capture weak signal. - Iter 2: `defer`; mitigated SIGINT/SIGTERM during `ssh-agent` startup with signal masking. - Iter 3: `defer`; tech reviewers are 3/3 OK. Remaining high dissent is `PR description: none`, but the PR body exists via Forgejo API. Terminal action: `defer_to_issue`. Tracked follow-up: #85 (`fix(canary): include PR body in reviewer context`). I am not running a 4th canary because the cap is reached.

codex added 1 commit 2026-05-05 23:25:52 +02:00

Merge remote-tracking branch 'origin/main' into codex/issues/79-startup-cleanup 2dfdbd661e

codex added 1 commit 2026-05-05 23:28:30 +02:00

Merge remote-tracking branch 'origin/main' into codex/issues/79-startup-cleanup ... 8624f23eef

# Conflicts:
#	control-plane/platformctl/tests/test_agent_access_ssh_agent.py

codex added 2 commits 2026-05-05 23:34:15 +02:00

Merge remote-tracking branch 'origin/main' into codex/issues/79-startup-cleanup a95e4f8213

Merge remote-tracking branch 'origin/codex/issues/79-startup-cleanup' into codex/issues/79-startup-cleanup 339ff91cef

pdurlej merged commit 646017b1ec into main 2026-05-05 23:39:38 +02:00

pdurlej referenced this pull request from a commit 2026-05-05 23:39:41 +02:00

Merge pull request 'fix(agent-access): harden ssh-agent startup cleanup' (#84) from codex/issues/79-startup-cleanup into main

codex referenced this pull request 2026-05-05 23:43:35 +02:00

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

codex referenced this pull request 2026-05-05 23:49:24 +02:00

fix(ci): add Infisical canary bridge #91

claude referenced this pull request 2026-05-17 17:27:24 +02:00

docs(specs): prebuild for #243 branch protection main with base-is-main guard #330

codex referenced this pull request from a commit 2026-05-17 21:30:16 +02:00

docs(specs): bug prebuild for #85 canary PR body in reviewer context

claude referenced this pull request from a commit 2026-05-18 08:16:30 +02:00

docs(specs): #85 bug prebuild — canary PR body in reviewer context

claude referenced this pull request 2026-05-18 08:16:51 +02:00

docs(specs): #85 bug prebuild — canary PR body in reviewer context #363

codex referenced this pull request 2026-06-10 23:39:51 +02:00

ci(patchwarden): publish contract-pr artifacts #782

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

W6d-automerge-calibration

  

agent/claude-code

Vistula actor: Claude Code.

  

agent/codex

Vistula actor: Codex.

  

agent/hermes

Vistula actor: Hermes Upstream.

  

agent/iskra

Vistula actor: Iskra.

  

agent/ollama

Vistula actor: Ollama or compatible model.

  

agent/patchwarden

Vistula actor: Patchwarden.

  

automerge-candidate

Candidate for narrow W6d autonomous merge readiness pilot

  

class/security-sensitive

Security-sensitive class of service: smallest coherent PRs and full canary

  

cutover-gate

blocks production declare / final RS2000 replacement

  

dependency/blocked

Vistula dependency state: blocked.

  

dependency/blocks-others

Vistula dependency state: blocks other work.

  

dependency/cross-repo

Vistula dependency scope: cross-repository.

  

dependency/needs-confirmation

Vistula dependency state: needs confirmation.

  

domain:agents

Agent identity, delegation, subagents, routing, or workflows.

  

domain:ci

CI, checks, Actions, runners, or release automation.

  

domain:docs

Documentation, packets, practices, or source-of-truth text.

  

domain:forgejo

Forgejo issues, PRs, labels, repo settings, or identity.

  

domain:infra

Infrastructure, storage, backup, DNS, network, or host substrate.

  

domain:memory

Memory, recall, write gates, salience, or Honcho-adjacent behavior.

  

domain:runtime

Runtime services, deploys, daemons, timers, or live behavior.

  

domain:signal

Signal UX, delivery, context, or outbound behavior.

  

domain:ux

Product UX, operator experience, or conversation ergonomics.

  

flow/architecture

Vistula lifecycle: architecture phase in progress.

  

flow/blocked

Vistula lifecycle: blocked on dependency or decision.

  

flow/deployed

Vistula lifecycle: deployed to runtime where applicable.

  

flow/done

Vistula lifecycle: work merged or otherwise done.

  

flow/implementation

Vistula lifecycle: implementation phase in progress.

  

flow/intake

Vistula lifecycle: incoming signal not refined yet.

  

flow/maintained

Vistula lifecycle: maintained/steady state.

  

flow/observed

Vistula lifecycle: observed after delivery.

  

flow/ready

Vistula lifecycle: ready for architecture or implementation.

  

flow/refining

Vistula lifecycle: Hermes/spec refinement in progress.

  

flow/retired

Vistula lifecycle: retired or intentionally closed.

  

flow/review

Vistula lifecycle: review phase in progress.

  

iterating

Orchestrator/Codex actively amending in response to review — operator can ignore until resolved

  

judge/codex-candidate

Candidate for Codex implementation.

  

judge/hermes-candidate

Candidate for Hermes discovery/research.

  

judge/low-confidence

Judgment confidence is low.

  

judge/needs-refinement

Needs clearer evidence or scope.

  

judge/operator-needed

Needs Piotr decision or input.

  

judge/p0

Urgent, operator attention or unblock now.

  

judge/p1

High value, schedule soon.

  

judge/p2

Useful, normal queue.

  

judge/p3

Low priority, keep but do not chase.

  

judge/park

Parked from current attention.

  

judge/patchwarden-candidate

Needs Patchwarden merge-safety attention.

  

judge/stale-priority

Existing priority judgment appears stale.

  

kind/adr

Vistula work kind: architecture decision record.

  

kind/bug

Vistula work kind: bug fix.

  

kind/chore

Vistula work kind: chore.

  

kind/feature

Vistula work kind: feature.

  

kind/infra

Vistula work kind: infrastructure.

  

kind/ops

Vistula work kind: operations.

  

kind/refactor

Vistula work kind: refactor.

  

kind/research

Vistula work kind: research/discovery.

  

large-impact

Per charter §3 review-by-impact: large impact (≥300 LOC OR new module OR identity/secret change OR architectural pivot) — Oracle pre-review recommended

  

merge/auto

Vistula merge mode: automated merge.

  

merge/manual

Vistula merge mode: manual merge.

  

merge/manual-dependency-conflict

Manual merge reason: dependency conflict.

  

merge/manual-failing-tests

Manual merge reason: failing tests.

  

merge/manual-merge-conflict

Manual merge reason: merge conflict.

  

merge/manual-missing-review

Manual merge reason: missing review.

  

merge/manual-operator-preference

Manual merge reason: operator preference.

  

merge/manual-red-zone

Manual merge reason: red-zone risk.

  

merge/manual-security-sensitive

Manual merge reason: security-sensitive work.

  

merge/manual-unclear-scope

Manual merge reason: unclear scope.

  

merge/manual-unknown

Manual merge reason: unknown.

  

meta

meta-decomposition issue; first agent decomposes into atomic children

  

mode:operator-only

Apply step requires explicit operator approval.

  

mode:patchwarden-iskra-approved

Work eligible for Patchwarden/Iskra approval inside repo policy.

  

mode:safe-auto

Low-risk work eligible for lightweight autonomous handling.

  

needs-operator-decision

PR is blocked on operator yes/no decision — fresh-Claude/Codex won't make this call alone

  

needs-triage

decomposing agent has open questions / unknowns

  

not-ready

reviewed but blocked on a precondition

  

observed/erroring

Vistula observation: erroring.

  

observed/needs-followup

Vistula observation: needs follow-up.

  

observed/pending

Vistula observation: pending.

  

observed/retire-candidate

Vistula observation: retire candidate.

  

observed/unused

Vistula observation: unused.

  

observed/used

Vistula observation: used.

  

operator-emotional

operator-emotional importance (Iskra memory etc.)

  

owner-attention

owner must decide; blocks default path

  

phase/02

Phase 02 cataloging

  

phase/03

Phase 03 control plane (platformctl)

  

priority:p0

Immediate safety, uptime, data, or operator-blocking issue.

  

priority:p1

Important active work; should be pulled soon.

  

priority:p2

Useful normal-priority work.

  

priority:p3

Opportunistic cleanup, research, or backlog work.

  

proposed

auto-generated by meta-decomposition; awaiting human review

  

ready-for-agent

reviewed and pickable; agents may claim per cookbook

  

ready-for-operator

PR is ready for operator review/merge — orchestrator finished its part

  

recovery

disaster recovery / restore semantics relevant

  

review:claude-reviewed

Reviewed by Claude-family agent.

  

review:codex-reviewed

Reviewed by Codex-family agent.

  

review:dziadek-reviewed

Reviewed by Dziadek pinch-point reviewer.

  

review:needs-human

Needs human review before merge or apply.

  

risk/exposure

incorrectly public/private/tailnet/auth/routed

  

risk/process

workflow/review/orchestration may produce bad outcomes

  

risk/product

work may have lost owner/user value

  

risk/runtime

platform may not run, recover, validate, behave reproducibly

  

safety:external-write

May write to external systems, users, rooms, repos, or services.

  

safety:no-prod-mutation

Must not mutate production or live external state.

  

safety:prod-impact

May affect production/runtime behavior.

  

safety:secret-touch

Touches secrets, credentials, auth, or secret-adjacent config.

  

size/large

Vistula size: large.

  

size/medium

Vistula size: medium.

  

size/small

Vistula size: small.

  

size/tiny

Vistula size: tiny.

  

size/unknown

Vistula size: not classified yet.

  

source/adr

Vistula source: ADR.

  

source/agent-generated

Vistula source: agent generated.

  

source/manual

Vistula source: manual entry.

  

source/operator-chat

Vistula source: operator chat.

  

source/voice-note

Vistula source: voice note.

  

status:blocked

Blocked by missing dependency, evidence, access, or operator decision.

  

status:codex-ready

Ready for a Codex implementation pass.

  

status:merged:pending-evidence

Merged but waiting for live or artifact evidence before closure.

  

status:needs-evidence

Needs proof, logs, tests, or operator-visible evidence.

  

status:operator-needed

Requires explicit operator input before safe progress.

  

status:parked

Intentionally deferred; not active campaign work.

  

tier/full

Full review tier per ADR-0007

  

tier/lite

Lite review tier per ADR-0007

  

tier/stacked

Stacked PR (base != main) escape hatch per ADR-0017. Requires consolidator-PR plan in PR body.

  

tier:0-platform-substrate

Data, secrets, backup, storage, or critical platform substrate.

  

tier:1-iskra-value-layer

Iskra/OpenClaw behavior, memory, runtime, or value-layer work.

  

tier:2-tools-products-modules

Tools, products, modules, experiments, and lower-blast-radius work.

  

type:bug

Incorrect behavior or regression.

  

type:chore

Maintenance, cleanup, metadata, or operational task.

  

type:docs

Documentation-only work.

  

type:feat

New capability or product behavior.

  

type:policy

Governance, protocol, boundary, or decision-contract change.

  

type:research

Investigation, spike, or evidence-gathering task.

No labels

W6d-automerge-calibration

agent/claude-code

agent/codex

agent/hermes

agent/iskra

agent/ollama

agent/patchwarden

automerge-candidate

class/security-sensitive

cutover-gate

dependency/blocked

dependency/blocks-others

dependency/cross-repo

dependency/needs-confirmation

domain:agents

domain:ci

domain:docs

domain:forgejo

domain:infra

domain:memory

domain:runtime

domain:signal

domain:ux

flow/architecture

flow/blocked

flow/deployed

flow/done

flow/implementation

flow/intake

flow/maintained

flow/observed

flow/ready

flow/refining

flow/retired

flow/review

iterating

judge/codex-candidate

judge/hermes-candidate

judge/low-confidence

judge/needs-refinement

judge/operator-needed

judge/p0

judge/p1

judge/p2

judge/p3

judge/park

judge/patchwarden-candidate

judge/stale-priority

kind/adr

kind/bug

kind/chore

kind/feature

kind/infra

kind/ops

kind/refactor

kind/research

large-impact

merge/auto

merge/manual

merge/manual-dependency-conflict

merge/manual-failing-tests

merge/manual-merge-conflict

merge/manual-missing-review

merge/manual-operator-preference

merge/manual-red-zone

merge/manual-security-sensitive

merge/manual-unclear-scope

merge/manual-unknown

meta

mode:operator-only

mode:patchwarden-iskra-approved

mode:safe-auto

needs-operator-decision

needs-triage

not-ready

observed/erroring

observed/needs-followup

observed/pending

observed/retire-candidate

observed/unused

observed/used

operator-emotional

owner-attention

phase/02

phase/03

priority:p0

priority:p1

priority:p2

priority:p3

proposed

ready-for-agent

ready-for-operator

recovery

review:claude-reviewed

review:codex-reviewed

review:dziadek-reviewed

review:needs-human

risk/exposure

risk/process

risk/product

risk/runtime

safety:external-write

safety:no-prod-mutation

safety:prod-impact

safety:secret-touch

size/large

size/medium

size/small

size/tiny

size/unknown

source/adr

source/agent-generated

source/manual

source/operator-chat

source/voice-note

status:blocked

status:codex-ready

status:merged:pending-evidence

status:needs-evidence

status:operator-needed

status:parked

tier/full

tier/lite

tier/stacked

tier:0-platform-substrate

tier:1-iskra-value-layer

tier:2-tools-products-modules

type:bug

type:chore

type:docs

type:feat

type:policy

type:research

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

pdurlej/platform!84

No description provided.