Repository navigation
feat: plan approval, stdin-file, two-phase once grants, JSON trimming - #6
Merged
Merged
Conversation
Request gains an optional Stdin byte payload, fed to the remote command's standard input by the shell-out runner (exec.Cmd.Stdin) and the native transport (ssh.Session.Stdin). A nil payload preserves the historical /dev/null behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Record gains stdin_sha256/stdin_bytes (payload identity without content) and plan_id (links approval lifecycle events minted by one plan submit). All three are omitempty and appended after the approval fields in both Record and canonicalRecord, so pre-upgrade records keep a byte-identical canonical form; a golden + per-field tamper test guards the chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…provals, run output trimming Field-test feedback batch (one deployment cost ~10 operator round-trips); this lands the approval-side fixes and features: - Once grants now consume in two phases: Authorize claims the grant under the request id (in-lock, so concurrent runs can never double-spend), and the claim settles after the execution outcome is known — a remote exit commits it, a transport-level failure releases it so the approval survives a clean re-run. Local cancel is intercepted (SIGINT/SIGTERM context) so the failed attempt is audited and the claim released. Deliberate trade-off: a connection dropped mid-execution also restores the grant even though the command may have run; both attempts stay in the audit log. - run --stdin-file streams a local file (<=32MiB) to the remote stdin. Approval store and audit record only sha256+size; gray-zone requests are forced exact and non-promotable, persistent host rules never match stdin runs, and grants are pinned to the exact content hash. - agentssh plan submit/status/wait/grant/deny bundle one task's gray-zone commands into a single operator review. Approving a plan mints one exact once/session grant per command (host scope deliberately not offered); execution still goes through run -> Authorize per command, so audit granularity and explicit-deny precedence are unchanged. Members reaped after resolution report expired, never denied. The TUI approvals tab shows plan membership and adjudicates a whole plan with [p]; stdin requests show their hash/size and hide host-allow. - run --json now truncates the echoed cmd at 2KiB (cmd_truncated) and always carries cmd_sha256; --fields projects the response onto the requested keys. stdin identity is stamped centrally on every response branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… design notes SKILL.md makes policy test a hard pre-check step, documents the plan approval round-trip and run --stdin-file, and notes the cmd echo truncation / --fields projection. Design records for both features live in docs/plans/plan-approval.md and docs/plans/run-stdin.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four fixes from the branch review: - authorize: when async approval is disabled, still strip generated __agentssh_approval host rules for stdin runs, so a rule that only matches command text can never authorize an unreviewed stdin payload. Operator-authored allow/deny rules are untouched. - audit: raise the JSONL read buffer (bufio.Scanner) above any accepted record so a >64 KiB command no longer breaks the append re-read/verify chain (which would drop the completed record and skip once-grant settle). - run --stdin-file: reject non-regular files (/dev/zero, FIFOs) and enforce the 32 MiB cap during the read via io.LimitReader, instead of reading to EOF before checking size. - ApplyDecision: append the approval_granted audit record before the durable grant, so no usable grant can exist without its audit record; ApplyPlanDecision inherits this per member. Tests: TestAuthorizeDisabledApprovalSkipsApprovalHostRulesForStdin, TestAuthorizeDisabledApprovalKeepsOperatorAllowRuleForStdin, TestApplyDecisionAuditFailureDoesNotCreateGrant, TestApplyPlanDecisionAuditFailureDoesNotCreateGrant, TestAppendVerifyLargeCommandRecords, TestLoadStdinSpecRejectsNonRegularFile, TestLoadStdinSpecRejectsOversizeDuringRead, TestLoadStdinSpecReadsRegularFile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-approval ban Align SKILL.md with the hardening round: plan wait/status can return exit 2 (expired records, re-submit) and plan grant/deny are operator-only verbs the agent must never self-invoke. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Field-test feedback batch: a real deployment cost ~10 operator approval round-trips. This branch cuts that to ~2-3 without weakening the trust model or losing per-command audit granularity.
Features
agentssh plan submit <host> --session <id> -- 'cmd1' 'cmd2' …(or--file) bundles a task's gray-zone commands into one operator review. Approving mints one exact once/session grant per command (no host scope in batch); execution still runs per command throughrun → Authorize, so audit granularity and explicit-deny precedence are unchanged. Addsplan status/wait/grant/denyand a TUI[p]whole-plan verdict. Members reaped after resolution reportexpired, neverdenied.run --stdin-file <path>— streams a local file (≤32 MiB) to the remote command's stdin, replacingprintf-quoting and sidestepping the LinuxMAX_ARG_STRLENargument limit. Content never enters the approval store or audit log (onlystdin_sha256+stdin_bytes); gray-zone stdin requests are forced exact and non-promotable, persistent host rules never match them, and grants are pinned to the content hash.runintercepts SIGINT/SIGTERM so cancellation is audited. Documented trade-off: a mid-execution disconnect releases the grant even though the command may have run.run --jsontruncates the echoedcmdat 2 KiB (cmd_truncated, full command stays in audit; correlate viacmd_sha256), and--fieldsprojects the response onto requested keys.policy testas a pre-check — SKILL.md makes it a hard pre-flight step, especially batch-checking before a multi-step change.Hardening (review-driven)
An 8-angle self-review plus a Codex pass surfaced four gaps, all fixed with tests:
__agentssh_approvalhost rules for stdin runs (a rule matching command text could otherwise authorize an unreviewed payload);--stdin-filerejects non-regular files and enforces the 32 MiB cap during the read;ApplyDecisioncommits theapproval_grantedaudit record before any durable grant, so no usable grant can exist without its audit record (plans inherit this per member).Verification
go build ./... && go test ./...green across all packages; real-binary smoke of policy test / plan submit / status / stdin / audit verify; TUI screens eyeballed. Design records indocs/plans/plan-approval.mdanddocs/plans/run-stdin.md.🤖 Generated with Claude Code