Skip to content

Protect behavior-defined engine session capture - #69586

Merged
pelikhan merged 20 commits into
mainfrom
copilot/protected-session-capture
Oct 25, 2026
Merged

pelikhan merged 20 commits into
mainfrom
copilot/protected-session-capture

Conversation

Copilot AI commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

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:

protected-session-capture: true

Unprotected execution remains unchanged. Includes schema validation, engine-authoring/reference documentation, adversarial regression coverage, and a patch changeset.

Capture and publication contract

  • Invoke native --capture-command-output with 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.
  • Parse only the final started attempt's stderr. Validate private regular files, ownership/permissions, metadata, drained completion, aggregate byte limits, and the final command exit status. Missing, incomplete, truncated, or inconsistent capture fails closed without falling back to stdout, prior attempts, runner presentation, or sandbox-written shared logs.
  • Require nonempty, structurally valid session events. Publish redacted structured events and normalized metrics, never raw native streams. Inspect mask requests from both streams solely for redaction; redact configured secrets and permitted tool/text payloads while rejecting masking that would alter session structure or accounting fields.
  • Run the protected parser before redaction, safe-output ingestion, and artifact publication. Bind configured secret names and their values directly to the trusted parser step, including secret references in top-level workflow environment configuration, so canonical artifacts and summary previews are redacted before publication.
  • Replace the shared exit-code sidecar with AWF's verified status and discard unverified shared logs/error artifacts. Protected outcome collection requires the canonical session and disables shared stderr, native-log, and error-sidecar recovery. Missing or invalid canonical evidence fails ingestion; both unified and fallback session-artifact uploads require parser success.
  • Explicitly exec the 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 /proc ancestry 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 main through 663912b6f1 in 22fb14ffc3. Security hardening is included in 56e87e1661.

  • Built the merged compiler and recompiled all 348 workflows without additional generated changes.
  • Focused Go regressions cover protected execution, standalone launchers, secret bindings, parser ordering, and publication gates; impacted Go unit tests passed.
  • JavaScript typechecking and six targeted runtime suites (613 tests) passed for the security hardening. The latest review follow-up also passed the protected reader (32 tests) and parser bootstrap (60 tests) suites and focused engine/AWF Go tests.
  • Formatting, Go/custom Go/JavaScript linting, schema freshness, and workflow drift checks passed after isolating the shared linter temporary directory.
  • All five reviewed threads received individual dispositions and were resolved; no additional code changes were warranted. No GitHub workflows were dispatched. Live Linux/AWF and producer-authentication acceptance remain outstanding.

✨ PR Review Safe Output Test - Run 39046126054

💥 [THE END] — Illustrated by Smoke Claude · claude · sonnet46 · 106.4 AIC · ⌖ 7.22 AIC · ⊞ 922 · ◷
Comment /smoke-claude to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥 [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.

@github-actions

Copy link
Copy Markdown
Contributor

Great work on protected session capture! 🎯 Focused change with tests and docs. This looks ready for review.

Generated by ✅ Contribution Check · copilot · auto · 39.8 AIC · ⌖ 0.635 AIC · ⊞ 10.6K · ◷

@dsfaccini

Copy link
Copy Markdown

This reply was posted by Codex Desktop using gpt-daybreak-blue-latest on behalf of David.

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 ${RUNNER_TEMP} destination also is not a sufficient boundary because that path can resolve inside an agent-visible mount. github/gh-aw-firewall#10166 records the boundary requirements, and github/gh-aw-firewall#10169 implements the needed --capture-command-output primitive.

Proposed contract for this PR:

  1. Pin the first AWF release containing 85a24fe94f11f138a1c52a031e0dc80ebd1ac16a and invoke its protected command capture.
  2. Select attempt-*.stderr.bin through AWF's attempt metadata after execution. Preserve partial bytes and the actual exit outcome, and fail closed for missing, failed, or truncated capture.
  3. Feed only that stderr capture to the engine log parser. Never parse captured stdout or /tmp/gh-aw/agent-stdio.log.
  4. Keep the documented runtime limits: non-TTY Docker/gVisor execution; reject unsupported TTY, external-microVM, and Docker-socket modes.

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 copilot/protected-session-capture, but GitHub returned 403 Permission to github/gh-aw.git denied to dsfaccini, so I cannot apply this correction to the PR branch directly.

@dsfaccini

Copy link
Copy Markdown

This comment was posted by Codex Desktop using gpt-6-sol on behalf of David.

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 /proc/<parent>/fd/2 for both reading and writing. The proposed Pydantic launcher also has a pipe on fd 3 carrying keyed records. Marking that descriptor non-inheritable does not prevent a tool subprocess from reopening it through /proc, reading the key, and writing a matching record. AWF's host-only file protects captured bytes after collection, not who wrote bytes into the container-side pipe; AWF's capture issue states this distinction explicitly.

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.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot update documentation, specification based on current feature

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Comment thread actions/setup/js/log_parser_bootstrap.cjs
Comment thread actions/setup/js/protected_command_output.cjs
Comment thread pkg/workflow/behavior_defined_engine.go
Comment thread pkg/workflow/engine_definition.go
Comment thread actions/setup/js/session_artifact.cjs
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
  2. Review (actions/setup/js/log_parser_bootstrap.cjs:375): Prompt injection risk: Protected capture publishes structured session events derived from untrusted native stderr without enforcing authenticated framing/sequence/terminal completeness. In protected mode, the only hard check here is that parseLog returns a non-empty logEntries array, so a malicious or spoofed recorder stream can inject forged events that get written to agent-session.jsonl and influence downstream session collection/metrics (protected stderr is selected via readProtectedCommandOutput). - Protect behavior-defined engine session capture #69586 (comment)
  3. Review (actions/setup/js/protected_command_output.cjs:41): Prompt injection risk: readProtectedCommandOutput validates AWF capture metadata and file properties but does not (and cannot) authenticate the guest-side producer. The returned stderr content is later treated as session evidence for protected ingestion, so without an additional deterministic verification layer (signatures/continuity/terminal completeness) a same-UID tool/engine can emit forged session frames that the parser will accept. - Protect behavior-defined engine session capture #69586 (comment)
  4. Review (pkg/workflow/behavior_defined_engine.go:648): Prompt injection risk: Enabling engine.behaviors.execution.protected-session-capture sets GH_AW_PROTECTED_SESSION_CAPTURE/GH_AW_PROTECTED_CAPTURE_ID and switches parsing to a runner-temp protected capture path, but there is no compiler/runner-side enforcement that the engine's log-parser actually verifies an authenticated session protocol. This makes it easy to enable protected ingestion while still accepting forged stderr-derived session events. - Protect behavior-defined engine session capture #69586 (comment)
  5. Review (pkg/workflow/engine_definition.go:233): Prompt injection risk: The new protected-session-capture engine behavior flag enables a mode where session events are derived from captured stderr; however the contract requires producer authentication and continuity checks, and the implementation path does not enforce that such verification happens before returning/publishing events. This leaves an opt-in configuration that can appear "protected" while still permitting forged session evidence. - Protect behavior-defined engine session capture #69586 (comment)
  6. Review (actions/setup/js/session_artifact.cjs:95): Prompt injection risk: writeSessionArtifact persists canonical session events to agent-session.jsonl. In protected-capture mode these events come from parsing untrusted captured stderr; without enforced protocol verification earlier in the pipeline, forged frames can be serialized and published as if they were trusted structured session evidence. - Protect behavior-defined engine session capture #69586 (comment)

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
Sous-chef work: 523858fac0889d43383f8381ae8b505bc4bf0ae0e399e88f5581e33a17b81ba7 678dc867db4ecde77b81274bbbc158145576f62afacd3a2c6540afa21fe40306 79fcf0c0a41daef8aed6ddc850e99e1b91c2b5452270cfd86515b624b367a4c5 81c20231252c516f166b5a6a2eda40f59a1c57d11368f7ae8ca1f8d3e372c5bd 827ed00c735b2c44f3b83480422310196db611338808cc1fd791a67a6e40470b
Sous-chef state: b12a817da51f5fa08ce9245f3dbed496e77d1bbfd3998eecfc30457ae41dcac6

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · ~11 AIC · ⌖ ~10 AIC · ⊞ 1.1K · ◷
Comment /souschef to run again

Copilot AI and others added 2 commits October 24, 2026 16:29
…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>

Copilot AI commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes....

Merged the latest main and committed c4a1080 with fail-closed structural validation for protected session events and regression tests. The suggested digest, harness-presence, config-flag, and marker checks cannot establish producer authenticity; authenticated framing, continuity, and terminal verification remain unresolved acceptance gates. The related review threads remain open.

Comment thread pkg/workflow/compiler_yaml_artifacts.go Outdated
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
  2. Review (pkg/workflow/compiler_yaml_artifacts.go:99): Sensitive information disclosure risk: when protected session capture is enabled, injectProtectedCaptureSecretEnvironment() injects GH_AW_SECRET_NAMES and SECRET_*=${{ secrets. }} into the 'Parse agent logs for step summary' github-script step environment, but that step runs an engine-provided behaviors.log-parser module. If a workflow imports a third-party/externally maintained engine definition, this expands secret exposure to untrusted parser code with no demonstrated sandboxing/egress control; prefer keeping secret values confined to compiler-owned redaction code or enforcing a trust/allowlist boundary for parsers. - Protect behavior-defined engine session capture #69586 (comment)

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
Sous-chef work: 562830a9825673886c98b74d6eaec7b030110731d87c8045b81de4cbd64c1d42
Sous-chef state: 06436af74991386f19bde5de63dca3179869e23bdb79b3e5359878bb35380dcb

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · ~9 AIC · ⌖ ~8 AIC · ⊞ 1.1K · ◷
Comment /souschef to run again

Copilot AI and others added 2 commits October 24, 2026 17:59
…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>
pelikhan and others added 2 commits October 24, 2026 20:12
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>
Comment thread pkg/workflow/compiler_yaml_ai_execution.go
Comment thread pkg/workflow/compiler_yaml_ai_execution.go
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan merged commit 7e8e11a into main Oct 25, 2026
49 checks passed
@pelikhan
pelikhan deleted the copilot/protected-session-capture branch October 25, 2026 04:30
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.93.4

@dsfaccini

Copy link
Copy Markdown

This comment was posted by Codex Desktop using gpt-6-sol on behalf of David.

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 umask 022, the filesystem probe creates a fresh work directory as 0755, then AWF's permission check rejects it before the agent launches.

The released compiler passes our checks for explicit exec, protected capture, configured-secret parser bindings, and both artifact-upload guards. Native compatibility acceptance for pydantic-ai #10606 remains pending. The new issue contains the reproduction and proposed directory-creation fix.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants