Skip to content

feat: Remediate hasna/machines#83 bounded-exec review blockers - #84

Merged
andrei-hasna merged 4 commits into
mainfrom
factory/20945ea8-b45b-4ffa-a8b7-9a696523
Aug 9, 2026
Merged

feat: Remediate hasna/machines#83 bounded-exec review blockers#84
andrei-hasna merged 4 commits into
mainfrom
factory/20945ea8-b45b-4ffa-a8b7-9a696523

Conversation

@hasna-identity

@hasna-identity hasna-identity Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Objective

Remediate #83 bounded-exec review blockers

Remediation cycle one for PR #83 exact reviewed tree. Add failing regression tests before production changes for exactly four named P1s: pre-materialization 65,536-character script input bound; collection-time stdout/stderr bound whose returned value including suffix obeys maxOutputChars; AWS_SECRET_ACCESS_KEY assignment redaction using synthetic fixtures only; and private machine route secrecy in unsafe-target errors. Apply the smallest owning fixes, preserve stream separation, exit propagation, timeout behavior, and safe shell quoting. Update the existing PR branch, include exactly one Agent: agent-ea trailer and no Co-Authored-By, run affected tests plus verify:release and staged secret scan, and return the exact new head to the same reviewer Feynman for focused re-review only. Do not merge, publish, install, use raw ssh, inspect real credential values, or broaden scope.

Verification

  • policy source: base 63cfb00 (immutable commit — agent-proof)
  • containment: env — allowlist env, non-login shell, run-scoped HOME (registry auth seeded for install)
  • install: pass
  • typecheck: pass
  • build: pass
  • test: pass
  • doctor (ci): ok — 11 checks passed (1 advisory)

Run run_6e5babcb9f79 · backend cursor · task 20945ea8-b45b-4ffa-a8b7-c86b43097560
🏭 Generated by @hasnaxyz/factory


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

BUG: @hasna/machines — no supported bounded remote execution primitive

Repro on station01 with @hasna/machines CLI 0.2.16 while auditing station02 for Hirefa.st task 4d7e47bb-6703-4d2a-9501-cbfa8bcdc10f: machines ssh --cmd COMMAND only prints a generated SSH command and does not execute it; machines command-matrix executes literal true; complete help exposes no package-owned arbitrary or bounded remote command runner. Raw ssh is forbidden, so HEAD, status, refs, stash, reflog, untracked files, and fsck cannot be safely verified. Acceptance: add or identify a package-owned bounded remote execution verb with explicit target, argv or script input, timeout, exit code, separated stdout and stderr, no credential output, and tests proving successful and failing remote probes; ship and install it, then rerun the exact Hirefa.st station02 audit.

X-Factory-Run: run_819d7d474ee0
X-Factory-Task: 203a3915-06e1-40ae-98cc-9f5f106d0d43

Agent: agent-ea
Remediate #83 bounded-exec review blockers

Remediation cycle one for PR #83 exact reviewed tree. Add failing regression tests before production changes for exactly four named P1s: pre-materialization 65,536-character script input bound; collection-time stdout/stderr bound whose returned value including suffix obeys maxOutputChars; AWS_SECRET_ACCESS_KEY assignment redaction using synthetic fixtures only; and private machine route secrecy in unsafe-target errors. Apply the smallest owning fixes, preserve stream separation, exit propagation, timeout behavior, and safe shell quoting. Update the existing PR branch, include exactly one Agent: agent-ea trailer and no Co-Authored-By, run affected tests plus verify:release and staged secret scan, and return the exact new head to the same reviewer Feynman for focused re-review only. Do not merge, publish, install, use raw ssh, inspect real credential values, or broaden scope.

X-Factory-Run: run_6e5babcb9f79
X-Factory-Task: 20945ea8-b45b-4ffa-a8b7-c86b43097560
@andrei-hasna

Copy link
Copy Markdown
Contributor

[REVIEW] NO_GO — #84 @ 1d16140 — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Reviewed the full origin/main...HEAD diff against freshly fetched base bc7123fac90af90e9a3844dc7489d46ca2ddb256, including every changed file and the surrounding CLI dispatch, remote runner/process-group timeout, SSH target validation, mutation-approval, redaction, and test code.

Commands and exact results:

  • bun install — exit 0; setup only, 116 packages installed.
  • bun run typecheck — exit 0; pass, no count emitted by tsc --noEmit.
  • bun run test — exit 0; 559 pass, 0 fail, 559 tests across 62 files.
  • Real-path control: bun src/cli/index.ts exec --machine local --timeout-ms 5000 -- sh -c 'exit 37' — exit 37. No approval token or trusted-local mutation opt-in was supplied, so the propagated 37 proves the command executed.

Blocking P0/P1 findings:

  • P1, high confidence — machines exec bypasses the package mutation-approval boundary (src/cli/index.ts:3220-3263). The action exposes no --approval-token and calls runMachineExec directly, unlike the repository's other mutating CLI paths. Source → flow → sink: unapproved argv or script → runMachineExecrunMachineCommand → local bash -c or remote ssh, allowing arbitrary filesystem, package, service, or data mutation. The real CLI control above confirms this is currently reachable at the reviewed head. Remedy: require the existing scoped machines_exec approval before invoking the runner, binding the exact machine, timeout, effective output cap, and argv or script; add denial, exact-token success, and tamper regressions.

Non-blocking follow-ups:

  • None.

@andrei-hasna

Copy link
Copy Markdown
Contributor

Fixed the named mutation-approval blocker in c4cb243.

  • Added the scoped machines_exec mutation operation and --approval-token CLI option.
  • Bound approval to the exact machine, timeout, effective output cap, and argv or script payload.
  • Added denial, exact-token success, changed-argv rejection, propagated failure, timeout, and script regressions.
  • Post-fix real-path control: the same unapproved command now prints Fleet mutation blocked: cli.machines_exec requires operator approval... and exits 1.
  • bun run typecheck — exit 0.
  • bun run test — exit 0; 559 pass, 0 fail, 559 tests across 62 files.
  • shield review on the staged four-file remediation — exit 0; No security issues found in staged changes.

@andrei-hasna

Copy link
Copy Markdown
Contributor

[REVIEW] GO — #84 @ e88dbf0c9d8a95ba3ac6d8d1e2746db8d550aa7f — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Focused re-review of the sole named blocking defect, its fix, and direct regressions.

Remediation:

  • machines exec now requires the existing scoped machines_exec mutation approval before runner invocation.
  • Approval binds the exact machine, timeout, effective output cap, and argv or script payload.
  • The real unapproved CLI control now prints the approval-required refusal and exits 1; exact-token success and changed-command rejection are covered by regression tests.
  • e88dbf0c9d8a95ba3ac6d8d1e2746db8d550aa7f reconciles current main into the PR. Its tree is byte-identical to the reviewed c4cb243593a27e20d16ed9c19cf25c4463b86fab tree.

Affected and required gates:

  • bun run typecheck — exit 0 after the fix; exit 0 again after conflict resolution.
  • bun run test — exit 0; 559 pass, 0 fail, 559 tests across 62 files on the exact tree now at the PR head.
  • shield review on the staged four-file fix — exit 0; No security issues found in staged changes.
  • Fresh merge-tree check against origin/main — exit 0; merge tree equals the PR head tree.

Blocking P0/P1 findings: none remain.

Non-blocking follow-ups: none.

@andrei-hasna

Copy link
Copy Markdown
Contributor

[REVIEW] GO — #84 @ e88dbf0 — lens: correctness+security+gates, reviewer Trebius (1 of 1)

This supersedes the immediately preceding GO comment, which expanded the short head SHA incorrectly and is therefore stale.

Focused re-review of the sole named blocking defect, its fix, and direct regressions:

  • machines exec now requires scoped machines_exec mutation approval before runner invocation, bound to the exact machine, timeout, effective output cap, and argv or script payload.
  • The real unapproved CLI control now prints the approval-required refusal and exits 1; exact-token success and changed-command rejection are covered by regression tests.
  • This head reconciles current main into the PR, and its tree is byte-identical to the reviewed and tested c4cb243593a27e20d16ed9c19cf25c4463b86fab tree.

Affected and required gates:

  • bun run typecheck — exit 0 after the fix and exit 0 after conflict resolution.
  • bun run test — exit 0; 559 pass, 0 fail, 559 tests across 62 files on the exact tree now at the PR head.
  • shield review on the staged four-file fix — exit 0; No security issues found in staged changes.
  • Fresh merge-tree check against origin/main — exit 0; merge tree equals the PR head tree.

Blocking P0/P1 findings: none remain.

Non-blocking follow-ups: none.

@andrei-hasna
andrei-hasna merged commit 8fdb66f into main Aug 9, 2026
2 checks passed
@andrei-hasna
andrei-hasna deleted the factory/20945ea8-b45b-4ffa-a8b7-9a696523 branch August 9, 2026 16:22
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