Repository navigation
Protect behavior-defined engine session capture - #69586
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · 106.4 AIC · ⌖ 7.22 AIC · ⊞ 922
Comment /smoke-claude to run again
Comments that could not be inline-anchored
actions/setup/js/log_parser_bootstrap.cjs:252
🔍 Smoke Test Review Comment (Run 39046126054)
Good use of the environment flag GH_AW_PROTECTED_SESSION_CAPTURE to gate this behavior. Consider adding an early return or log message when the flag is not set, so operators know the protected capture was skipped intentionally.
actions/setup/js/log_parser_bootstrap.test.cjs:41
🧪 Smoke Test Review Comment (Run 39046126054)
Solid test coverage for the protected session capture path. The use of vi.fn().mockReturnValue ensures deterministic behavior. Consider also testing the error path when fs.renameSync throws an exception to ensure the finally block cleans up correctly.
|
Great work on protected session capture! 🎯 Focused change with tests and docs. This looks ready for review.
|
This needs one security correction before it is mergeable: the protected input to the session parser must be AWF's host-only stderr capture, rather than a runner-side tee of the combined engine stream. The current branch still combines engine stdout and stderr. That lets ordinary child or tool output enter the parser input, including matching-key records if the key is observed. Its Proposed contract for this PR:
The corresponding Pydantic engine change puts the framing header and descriptor-3 recorder frames on engine stderr, marks descriptor 3 non-inheritable, removes the key before agent code starts tools, and routes both ordinary child stdout and stderr to engine stdout. I attempted a direct push to |
Correction to my earlier contract proposal: selecting AWF's host-only stderr capture is necessary, but it does not authenticate Pydantic's session records. I reproduced the remaining boundary failure in Docker: a same-UID child can reopen its parent's stderr pipe through Please keep this PR in draft while we establish a producer-authentication or process-isolation boundary that survives a same-UID tool process. Pydantic PR #10606 remains blocked on that end-to-end contract and a released gh-aw runtime. |
|
@copilot update documentation, specification based on current feature |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 890ced0
|
…sion-capture Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Merged the latest |
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: c4a1080
|
…sion-capture Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bind publication redaction secrets to the trusted parser, block shared diagnostic fallback, and gate both artifact transports on verified capture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
Released-runtime follow-up: AWF #10618 tracks a startup failure found while validating gh-aw v0.93.4 with its pinned AWF v0.28.70. Under The released compiler passes our checks for explicit |
Summary
Integrates @dsfaccini's protected-session-capture work using AWF's native protected command output, replacing the original runner-side combined-stream tee. Behavior-defined engines opt in under
engine.behaviors.execution:Unprotected execution remains unchanged. Includes schema validation, engine-authoring/reference documentation, adversarial regression coverage, and a patch changeset.
Capture and publication contract
--capture-command-outputwith a fresh run/attempt/job/matrix-scoped destination and a separate AWF work directory. AWF owns exclusive directory creation, mount isolation, bounded collection, teardown, and internal attempts; protected execution has no runner-side tee or outer startup retry.execthe final Node/direct launcher. Require an enabled Docker sandbox and AWF v0.28.67+; reject microVM runtimes, TTY, and raw DinD/runtime overrides. AWF preflight remains authoritative for effective mount aliases and Docker-socket exposure. The global AWF pin remains v0.28.70.Producer requirements and remaining acceptance gates
Native storage isolation does not authenticate a guest-side producer. The engine's trusted parser must verify framing/signatures, sequence continuity, and terminal completeness before returning events, without legacy fallback. A keyed prefix or first public-key header on an accessible pipe is insufficient. Structural guards, co-located unkeyed digests, parser-set booleans, and harness/configuration declarations are not substitutes for producer verification.
The existing pinned Pydantic keyed-stdout definition is not enabled for protected capture and is not silently rewritten. A compatible upstream recorder must emit session data on stderr, replace intervening launchers (for example with Node
process.execve()), protect its key and recorder descriptor through non-dumpability/process isolation, and route ordinary tool output to stdout. pydantic/pydantic-ai#10606 must satisfy and demonstrate that contract before adoption; this PR does not introduce a replacement signing protocol.An opt-in real-AWF/Linux regression exercises the generated launcher, a non-dumpable recorder, and a same-UID child probing
/procancestry and recorder descriptors. It has compiled but has not passed a live run here. Earlier attempts were blocked by the macOS host and a Linux Docker daemon with full snapshot storage; no shared Docker resources were pruned or isolation checks weakened. A successful Linux/AWF run and compatible upstream producer verification remain required acceptance evidence. Resolving the five reviewed implementation suggestions does not close these gates.Integration and validation
Merged
mainthrough663912b6f1in22fb14ffc3. Security hardening is included in56e87e1661.✨ PR Review Safe Output Test - Run 39046126054