Skip to content

Preserve server-bound session provenance in composed workflows - #9

Merged
jwilger merged 1 commit into
mainfrom
foundry/composed-session-provenance
Sep 27, 2026
Merged

jwilger merged 1 commit into
mainfrom
foundry/composed-session-provenance

Conversation

@jwilger

@jwilger jwilger commented Sep 27, 2026

Copy link
Copy Markdown
Member

Bug

When includeWorkflow() is present, the composition wrapper projects even root-node contexts via projectWorkflowContext(). That projection dropped originSessionId. The executing engine had a server-bound interactive Pi identity, but a root health action saw no identity; a nested action had the same defect. This breaks fail-closed session-bound integrations without making workflow input trustworthy.

Fix

Preserve only the engine-supplied context.originSessionId in projected root/child contexts. Leave the field absent for headless/unbound runs; never derive it from workflow input or a saved context/claim. The existing effect, signal, and settings projection are unchanged.

Verification

  • Added regression for root and nested actions in a graph with an include: authenticated session is visible at both, a fabricated input claim is not substituted, and unbound contexts stay unbound.
  • npm run format:check, npm run lint, npm run typecheck, npm run build: pass.
  • Full Vitest suite with NixOS-specific TMPDIR=/var/tmp, log.showSignature=false, --maxWorkers=4 --testTimeout=45000: 116 files / 1,439 tests passed. The unrestricted local run had 5 timeouts under heavy concurrent load; those 32 tests passed on bounded rerun before the full green run.
  • Disposable Pi profile reproduction on the already-merged dependency: a composed Foundry health action parked saying the server-bound session was absent despite a live interactive runner envelope containing it. A diagnostic overlay only of this source fix then reached the next expected fail-closed guard (missing Serena consent/receipt); the overlay is not a merged-product or all-integrations-health claim.

This PR only fixes context projection. It does not change the engine's session trust boundary or claim that arbitrary extensions/trusted Pi code are OS-isolated.

Context projection for includes discarded originSessionId even at the composed root, so an interactive action could not verify its live Pi session. Forward only the existing engine-bound identity through root and nested scopes; do not derive it from workflow input or projected state. Test bound and absent provenance in both scopes.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b780a261-eed0-46ba-9972-b65145761693

📥 Commits

Reviewing files that changed from the base of the PR and between 1462d34 and 22c7265.

📒 Files selected for processing (2)
  • src/workflows/composition.ts
  • test/action-session-provenance.test.ts

Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: e2e
  • GitHub Check: tui
  • GitHub Check: installed-e2e
  • GitHub Check: check
🔇 Additional comments (2)
src/workflows/composition.ts (1)

684-687: LGTM!

test/action-session-provenance.test.ts (1)

5-11: LGTM!

Also applies to: 73-134


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Composed workflows now preserve the active session ID in both root and nested action contexts. When no session is bound, the context remains null, and session IDs supplied as workflow input are not treated as active session IDs.

Walkthrough

Workflow context projection now forwards a defined engine-bound origin session ID. Tests cover root and nested action contexts for bound and unbound engines, including input that claims a different session ID.

Changes

Session provenance

Layer / File(s) Summary
Project engine-bound session provenance
src/workflows/composition.ts, test/action-session-provenance.test.ts
Context projection retains the origin session ID when defined. Tests check that root and nested action contexts receive the engine-bound value, or null when unbound, while a claimed input ID remains separate.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: osolmaz

Merge Risk: ⚪ Minimal · up to 22c72

No actionable merge-blocking risk is identified; the change appears ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 22c72

Composed actions now receive an origin session ID. A headless run can also be started with a supplied ID, so it is not yet clear that actions can distinguish that value from a live, session-bound identity. No misuse by an action was established.

Retained concerns

  • Medium · security · inferred: A headless composed action may receive a supplied origin session ID without a live interactive session binding. If an action treats that field alone as proof of session authority, the new projection expands where that distinction matters.
Security review details

Security Blast Radius

  • inferred — The immediate exposure is actions in composed root and included scopes of an affected run. No cross-tenant reach or privileged sink is established by the inspected evidence.

Security Findings and Attack Paths

  • inferred — If a headless launch conveys its supplied origin ID to the engine, a composed action will now see that ID. Abuse would additionally require an action to mistake it for authenticated interactive-session authority; no such sink was verified.

Trust Boundaries and Controls

  • observed — Workflow input does not populate the projected identity field: the projection copies context.originSessionId independently, and the test confirms a fabricated input claim remains separate.
  • observed — The runner's headless-executor choice is based on bootstrap.originSessionId, while the identity passed into its engine comes from launch.originSessionId. The inspected code does not prove these fields have identical binding semantics.

Resilience and Maintainability Implications

  • inferred — The unbound-engine regression protects absence of identity when no ID is supplied, but does not settle the distinct headless case in which a start request supplies one.

Hardening Proposals

  • proposed — Define the action-context field against the runner's verified interactive binding, and exercise a server-backed headless composed run with a supplied origin ID before treating field presence as authorization evidence.

Comment @coderabbitai help to get the list of available commands.

@jwilger
jwilger merged commit 131f74e into main Sep 27, 2026
6 checks passed
@jwilger
jwilger deleted the foundry/composed-session-provenance branch September 27, 2026 04:23
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