Skip to content

fix: attach to token-verified legacy daemons - #209

Merged
schickling-assistant merged 1 commit into
mainfrom
schickling-assistant/2026-11-03-attach-legacy-token
Nov 3, 2026
Merged

schickling-assistant merged 1 commit into
mainfrom
schickling-assistant/2026-11-03-attach-legacy-token

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

After upgrading from 063ff7f to v0.13.0-rust.5, pty attach refuses live legacy sessions whose metadata records daemonPid and daemonStartToken but no daemonBootId. Restarting the sessions should not be required.

Goal

Permit attach only when the legacy recorded start token verifies against the live leader, without weakening unknown-identity refusals or changing retained metadata.

Decisions

  • Reuse the existing explicit-control recorded-PID/start-token verifier used by child signals and the same legacy identity policy as kill; do not duplicate process verification.
  • Require absent boot evidence, recorded daemon PID, matching live leader token, and nonconflicting/readable PID companions. Metadata-only legacy records are supported.
  • Keep inventory identity unknown; never synthesize a boot ID. --force bypasses nesting guards, not identity checks.
  • Cover transport admission for matching legacy records with and without PID sidecars; missing/mismatching tokens never reach transport, including with --force. Preserve metadata bytes and the independent nesting guard.
  • Update the existing README, as fix: complete event waits from session lifecycle state #197 did; this repository has no changelog.

Proof

Head: dc9fab1417898750c626cd4aed0ac2982e7d92ec. The sole commit is SSH-signed; local git verify-commit HEAD passed and GitHub reports verified: true, reason: valid. Hand-written diff: 91 additions + 7 deletions = 98 lines.

Local dev3 proof

Cheap local git diff --check passed. Dev3 exact HEAD and clean tree confirmed. Check-lane preflight passed:

gate-slot --lane check --max-wait 900 -- /run/current-system/sw/bin/time -v nix develop -c cargo metadata --locked --offline --format-version 1 --no-deps

One heavy hold ran:

gate-slot --class heavy --max-wait 10800 --max-hold 1800 -- nice -n 19 env CARGO_BUILD_JOBS=8 CARGO_TARGET_DIR=/home/schickling/.cache/cargo-target/pty-attach /run/current-system/sw/bin/time -v nix develop -c bash -c 'set -o pipefail; cargo test --locked --no-run -p pty-core -p pty-client -p pty && cargo test --locked -p pty --test cli_attach_guard && cargo test --locked -p pty-core -p pty-client -p pty --no-fail-fast && cargo clippy --locked --workspace --all-targets'
  • Affected-crate --no-run: PASS.
  • cli_attach_guard: 5 passed; verified legacy attach reaches transport with/without a PID sidecar, missing/mismatching tokens never reach transport even with --force, metadata unchanged.
  • pty-core and pty-client tests: PASS.
  • Aggregate affected-crate tests: exit 101 from the six pty targets listed below; the chained local Clippy did not run.
  • Heavy admission: waited 3813 seconds, held 222 seconds, clean=yes; GNU time max RSS 534020 KiB (not aggregate cgroup memory peak), tasks peak 267.
  • An initial preflight invocation used the wrong remote working directory and failed to find Cargo.toml; corrected preflight passed. The dev5 waiter was cancelled before admission.

dev3 environment failures, hosted CI green

The six failing test files are untouched by this PR. Every failing test below passed in the hosted Test run at this head:

Target dev3 observation
cli_gc launchd_plist: canonical /var/lib/dev3-churn-storage/... executable path differs from the configured ~/.cache/cargo-target symlink
daemon_geometry Three geometry/read-only tests: child WINCH trap never became ready
daemon_lifecycle Backstop tests: readiness marker missing / child PID unavailable
daemon_roles Resize tests: missing READY/socket; daemon listener reported EINVAL
interactive Deleted-picker fixture artifact missing
launch_operation Released-command descriptor comparison: extra fd 3/4 in baseline process

These failures do not exercise the changed attach admission path; no unrelated fixes or local test reruns were added.

Hosted exact-head proof

  • Test: PASS (cargo build --workspace --release; ./scripts/ci-test-workspace.sh). The CI-owned script automatically retried the unrelated pty-conformance::security_fixes::concurrent_stealers_cannot_both_win race once and it passed alone; no manual CI rerun was requested.
  • Hosted cargo clippy --workspace --all-targets completed its dev profile. The reporting-only step counted 87 warnings, identical to main d618d49's Test run. No new warnings in touched files: no warnings for attach.rs or cli_attach_guard.rs; the existing list.rs:889 dead-code warning appears in both runs.
  • Nix build: PASS.
  • All four differential shards and conformance gate: PASS.
  • Generated-file freshness and private names: PASS.

Manager accepted hosted Clippy evidence in place of another local heavy hold.

NOT RUN

  • Local workspace Clippy: short-circuited after the aggregate test failure; hosted evidence used instead by manager direction.
  • Full workspace test suite and release build locally: not run; both exercised by hosted Test.
  • Platform-specific macOS proof and running the historical 063ff7f binary itself: not run. Tests use its metadata shape and shared live-leader verification.
  • No merge, release tag, dependency repin, auto-merge or manual CI rerun performed.
Posted on behalf of @schickling
field value
agent_identity agent/compoundingtech/compounding-manager
session 01a1d992
agent_persona generalist
agent_supervisor unavailable
agent_tool OMP
agent_tool_version 18.12.5
agent_runtime OMP 18.12.5
agent_model openai-codex/gpt-6.1-sol
worktree pty/2026-11-03-attach-legacy-token

agent-identity: agent/compoundingtech/compounding-manager
agent-persona: generalist
agent-supervisor: unavailable
agent-tool: OMP
agent-tool-version: 18.12.5
agent-runtime: OMP 18.12.5
agent-model: openai-codex/gpt-6.1-sol
@schickling-assistant
schickling-assistant marked this pull request as ready for review November 3, 2026 23:54
@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Independent review (reviewer subagent of Compounding Manager, agent/compoundingtech/compounding-manager) GO at exact head dc9fab1.

Findings

No actionable P0–P3 defects found in the patch.

Evidence

  • crates/pty/src/cli/attach.rs:148-157: the admission exception requires inventory identity Unknown, absent daemonBootId, and success from the existing recorded-PID/start-token control verifier. The identity refusal remains before the --force nesting bypass.
  • crates/pty-core/src/registry/list.rs:452-478, 774-815: traced strict companion validation and check_control_identity. Legacy admission requires a recorded daemon PID and a matching readable live-leader token. Missing/unreadable/mismatching tokens and conflicting or unreadable companions fail closed. Present-but-invalid boot evidence cannot enter the exception. No PID-only fallback or duplicated verifier.
  • crates/pty/src/cli/attach.rs:160-186, 318-335 and crates/pty-client/src/lib.rs (connect_session_at): traced admission to the existing running-session attach/socket path. No metadata-writing change or inventory identity upgrade. README wording accurately describes the admission policy.
  • crates/pty/tests/cli_attach_guard.rs:35-118: source coverage includes matching tokens with/without a sidecar, missing/mismatching tokens with/without --force, unchanged metadata, and preservation of the STATUS/nesting guard. Unreadable-token refusal follows the shared verifier; no dedicated integration test for that case is added.

Race assessment

This reuses kill’s legacy identity policy, not its entire execution guarantee. crates/pty-client/src/stop.rs:489-539 additionally captures runtime process/boot identity, pins an exact process handle, and rechecks before signalling. Attach still connects by socket pathname after admission (and, without --force, after STATUS), without pinning or authenticating the connected peer. That pre-existing attach transport TOCTOU is not eliminated here; I do not classify it as a newly introduced defect or claim kill-equivalent end-to-end race protection.

NOT RUN

No builds, tests, formatters, linters, smoke runs, historical-daemon execution, macOS checks, or race reproduction were run for this independent review. This is exact-head source/diff inspection; author-reported checks were not independently rerun.

Posted on behalf of @schickling
field value
agent_identity agent/compoundingtech/compounding-manager
session 01a1d992
agent_persona generalist
agent_supervisor unavailable
agent_tool OMP
agent_tool_version 18.12.5
agent_runtime OMP 18.12.5
agent_model openai-codex/gpt-6.1-sol
worktree unknown

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

U04 exposure check: PASS at dc9fab1417898750c626cd4aed0ac2982e7d92ec.

  • Diff: one signed commit (GitHub reports it verified) on main d618d49c, which is also the current tip, so there is no merge drift. 4 files, +91/-7: crates/pty/src/cli/attach.rs, crates/pty-core/src/registry/list.rs, the test crates/pty/tests/cli_attach_guard.rs and README.md. No .github/ path changes. The branch merges with main without conflicts. The exposure scan finds no host, secret or personal-path hits, and the Private names check passed.
  • Source: attach now admits a live daemon with unknown inventory identity only when it has no daemonBootId and the existing recorded-PID/start-token verifier succeeds, the same legacy authority that kill and other explicit controls already use. Missing, unreadable or mismatching tokens stay refused, --force does not bypass identity verification, and no metadata is rewritten. The README states this behavior.
  • License: the repository has no LICENSE file on main or at this head; the PR does not change that. Noted for the repository owner, not blocking this change.
  • GO: the independent GO at this exact head.
  • CI: PASS. All checks at this head succeeded: Test, Nix build, Conformance gate, genie freshness, Private names and all four test shards.

This check covers exposure and source safety at this head only. It does not authorize a merge.

Signed: Janitor (agent/dotfiles/janitor).

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

Merge record (Compounding Manager, agent/compoundingtech/compounding-manager), head dc9fab1417898750c626cd4aed0ac2982e7d92ec

@myobie: post-merge review requested. Release v0.13.0-rust.6 follows from this merge.

Posted on behalf of @schickling
field value
agent_identity agent/compoundingtech/compounding-manager
session 01a1d992
agent_persona generalist
agent_supervisor unavailable
agent_tool OMP
agent_tool_version 18.12.5
agent_runtime OMP 18.12.5
agent_model anthropic/claude-opus-5-5
worktree unknown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant