feat(#6689)!: run GPT on pi through OpenAI Workload Identity Federation - #6695
Conversation
|
🤖 Finished Review · ✅ Success · Started 7:06 PM UTC · Completed 7:26 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $11.81 |
Site previewPreview: https://c22307fd-site.fullsend-ai.workers.dev Commit: |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: elevated (3/5) DetailsElevated risk driven by large size (52 files, 5399 lines), 8 protected .github/ paths modified, security-critical modifications to credential handling and redaction infrastructure, and the security-labeled WIF domain. Partially offset by majority of added lines in new files with no regression surface, excellent issue specification, strong issue-to-PR alignment, no dependency changes, bot author, and familiar pattern mirroring existing provider gates. Previous runRisk Assessment: elevated (3/5) DetailsElevated risk driven by triple growth in size (2988 lines, 36 files), 7 protected .github/workflows/ paths now modified (previously untouched), security-critical modifications to sandbox.go (credential handling) and redactor.go (token masking), and the security-labeled domain (WIF token exchange, credential isolation). Partially offset by majority of added lines in new files with no regression surface, excellent issue specification with 8 acceptance criteria, strong issue-to-PR alignment, no dependency changes, and familiar pattern mirroring existing xai-vertex provider gate. Previous run (2)Risk Assessment: moderate (2/5) DetailsModerate risk driven primarily by large change size (995 lines, 13 files) offset by additive nature (new package, scaffold files), no protected paths or security-sensitive files touched, bot author, no CI/dependency changes, and strong issue-to-PR alignment. |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsHigh
Medium
Low
Next steps:
|
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/fullsend/fullsend/.fullsend/.fullsend-cache/resources/sha256/416e3495dbe6b3d0ece160ca879d45f30595f26b71c301116c148872b408d019/scripts/post-fix.sh failed: exit status 1) · Started 7:28 PM UTC · Completed 7:42 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.22 |
|
🤖 Review · Commit: |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/fullsend/actions/runs/33108479027 Details: |
|
/fs-fix-stop |
|
Fix agent disabled for this PR. Remove the |
58644c9 to
4d38de7
Compare
|
🤖 Review · Commit: |
4d38de7 to
5fb76bf
Compare
|
🤖 Finished Review · ✅ Success · Started 8:08 PM UTC · Completed 8:27 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $13.86 |
|
🤖 Finished Review · ✅ Success · Started 2:01 AM UTC · Completed 2:44 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
docs/guides/infrastructure/layered-config-reference.md(file-level): Line 325 · [low] missing-doc
Code defaults reference table does not include rows for inference.openai.audience, inference.openai.identity_provider_id, inference.openai.service_account_id.
Suggested fix: Add three rows with empty string defaults to match the Merge semantics table.
A WIF access token lives at most an hour and OpenAI issues no refresh token, so `fullsend run` now keeps a refresher per run-scoped provider: shortly before the recorded expiry (margin plus jitter, bounded retries) it requests a fresh GitHub assertion, re-exchanges it, hot-updates the provider through the no-expansion path (sandbox.UpdateProviderLiteral) and records the new expiry; a static OPENAI_API_KEY only has its provider expiry pushed out on the same schedule. When the retries are exhausted the loop stops and says so — the recorded expiry then makes the gateway fail closed instead of the run silently outliving its credential. The refresher is stopped before the deferred cleanup runs, so a late refresh can never resurrect an expired credential. This is the OpenAI half of #6464 Track D; the Vertex OIDC file refresher is left as it is. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The three FULLSEND_OPENAI_* repository variables (audience, identity provider id, service account id) reach the runner through an explicit `env:` entry on the `fullsend run` step of each reusable stage workflow and each inline stage job in reusable-dispatch.yml, the way the OTEL variables do. They are identifiers, not secrets: the `vars` context resolves against the caller repository inside a reusable workflow, so no forwarding hop or shim change is needed. Unset variables leave the openai provider unusable in that repo, which is the opt-in the pilot wants. TestOpenAIVariableForwarding pins every run step. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The same shape as features/runtime/pi.feature — a minimal tool-using agent, asserting the pi runtime, a tool call in the session transcript and token metrics — on openai/gpt-5.6-luna with the openai provider and no host files, so a successful run proves the secretless credential path end to end. Gated on runtime-pi-openai, which the Makefile deliberately does not declare until an OpenAI organization is mapped to the pool repositories. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
A user-facing infrastructure guide for GPT on pi without a stored key: what to create in the OpenAI console (organization-level provider, a claims check, the per-repository mapping with api.model.request), the three repository variables, how to put an agent on a GPT model, running locally with an env file, how it stays safe in plain terms, and an action-oriented troubleshooting table. The pi runtime page and the local-run guide point at it; the docs sidebar lists it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
… placeholder prefix OpenShell 0.0.110+ (PR #2493) resets any inspected request body that contains the literal placeholder prefix, whether or not a credential follows it — a substring test, applied to every inspected REST endpoint once the sandbox holds any static credential (NVIDIA/OpenShell#2904). An agent that has read a file, diff or issue mentioning the prefix could therefore not call the model: fullsend's own review agent failed on every head of this PR with ECONNRESET. Model request bodies never legitimately carry a placeholder, so the two default model profiles now set allow_uninspected_credentials on their endpoints. Verified on a 0.0.115 gateway: a body carrying the prefix gets 200, the bearer header is still injected, and the method/path rules still apply (GET /v1/models stays 403). The fleet's own Vertex profile lives in fullsend-ai/agents and needs the same flag. Assisted-by: Claude (fix), Codex (review), Grok (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
4967308 to
149e667
Compare
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 3:01 AM UTC · Completed 3:42 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $18.15 |
waynesun09
left a comment
There was a problem hiding this comment.
Approving after six review-squad rounds (Claude, Codex, Grok) plus fullsend's own review agent: every MEDIUM+ finding is fixed or explicitly accepted in ADR 0092, all review threads are resolved, required checks (behaviour, e2e) and the rest of CI are green on 149e667, and the static-key path was verified end to end on the pinned OpenShell 0.0.115 (three clean smoke runs on the final design).
The red dispatch / Review job is not a code issue: the review agent reconstructs the placeholder prefix while reasoning about it and OpenShell 0.0.110+ resets that request body (#6716, upstream NVIDIA/OpenShell#2904); the fleet's Vertex profile in fullsend-ai/agents needs the same allow_uninspected_credentials opt-out this PR applies to the default profiles.
Still gated on external access: the live WIF exchange and a mid-iteration rotation (first mapped repository is that check; failure mode and fallback recorded in the ADR).
Add allow_uninspected_credentials: true to the *.googleapis.com endpoint in profiles/fullsend-vertex-ai.yaml. OpenShell 0.0.110+ resets any inspected request whose body contains the literal gateway-placeholder prefix (NVIDIA/OpenShell#2904), which causes ECONNRESET failures when agents quote the placeholder namespace in model request bodies. This flag skips the request-body placeholder scan while preserving TLS termination, bearer header injection, and method/path rule enforcement. The scaffold copies were already patched in fullsend-ai/fullsend#6695; this is the fleet harness copy loaded from this repository. Closes #1080 Signed-off-by: Wayne Sun <gsun@redhat.com> Assisted-by: Claude (code), Codex (review), Grok (review)
That's ugly, why isn't just live updating the provider viable? This smells like a pi bug or design flaw that we can't just rely on substitution at the HTTP layer. |
Summary
Secretless GPT on the pi runtime (#6689, ADR 0092), end to end:
fullsend runobtains a short-lived OpenAI access token itself, keeps it in a run-scoped OpenShell provider that it refreshes and removes, and the sandbox only ever sees the gateway's placeholder. Credential-delivery tier 2 (ADR 0025) — no OpenAI credential, no GitHub OIDC token and no credential file inside the sandbox.Breaking change (
feat(#6689)!):${OPENAI_API_KEY}now expands to the empty string wherever a run reads a value from the runner environment — harness YAML,env.sandbox, provider definitions — and is stripped from pre/post scripts (the same treatment as the other runner-only variables). A harness that copied the key into the sandbox itself must declareproviders: [openai]and let the run-scoped provider carry it.Seven commits, one per plan item:
internal/inference/openaiwif,internal/runtime/pi_run.go,profiles/fullsend-openai.yaml,providers/openai.yaml, ADR 0092) — OpenAI Workload Identity Federation: GitHub OIDC assertion for the configured audience →POST https://auth.openai.com/oauth/token(JSON body,subject_token_typejwt,token_typevalidated as Bearer, HTTPS-only, GitHub OIDC host only, strict body bound, lifetime capped at 1 h, whitespace in the token rejected) → opaqueaccess_token+expires_in+scope. On pi: the placeholder the sandbox environment carries forOPENAI_API_KEYis written into pi'sauth.json(PiOpenAIAuthSeed) before the agent-writable.envis sourced, and pi starts without--api-key— pi'sAuthStoragere-reads that file on every revision change and resolves the key per request, which is what lets a running iteration follow a refresh (see "Verified against OpenShell 0.0.115"). The pi binary is pinned read-only before.env(readonly FULLSEND_PI_BIN) and launched by path; a config-dir integrity guard (exit 98 whenmodels.jsonexists, orauth.jsonis anything but pi's own{}or exactly the seeded placeholder entry; any\uescape rejected) runs before.envand again after it behindunset -f test command grep tr sed printf pi, withPI_CODING_AGENT_DIRand the offline switches re-exported in between;OPENAI_BASE_URL/AZURE_OPENAI_API_KEY/OPENAI_API_KEYare unset after.env.internal/cli/run_openai.go) — thefullsend-openaiprofile is imported from the scaffold embedded in the binary on every run (its id is reserved: a workspace or URL-resolved profile with that id is refused) and confirmed present viaprovider list-profiles -o json; a bareproviders: [openai]with noproviders/openai.yamlon disk falls back to the embedded definition (OpenAI only); credential source resolution (WIF when the three ids are set — runner variables, or the committedinference.openaiblock, never merged; a partial trio is an error; a token whose mapping grants more thanapi.model.request/api.model.readis refused;OPENAI_API_KEYfrom the runner environment for local runs, with a CI warning); exact-value redaction +::add-mask::;openai-<sandbox suffix>created empty through a no-expansion path (sandbox.EnsureProviderLiteral) and then given its value and--credential-expires-atin one update;OPENAI_API_KEYand theFULLSEND_OPENAI_*ids join the expansion deny list, which now also covers provider definitions (sandbox.DenyExpansionKeys). Egress preflight: before the agent starts, the run reads the sandbox's effective policy (stdout only, fails closed when unreadable) and refuses to continue when any rule admitsapi.openai.com:443without L7 inspection (noprotocol,protocol: tcp,tls: skip, a host-only rule, or anallow_uninspected_credentialsopt-in, which does not make the credential injectable either) — since OpenShell 0.0.110 the proxy will not inject a credential over such a route, and the base image's default policy ships exactly that in itscodexrule; the fleet'spolicies/base.yamlhas no network rules, so with it the provider's inspected entry is the only route. Cleanup deletes the provider (waiting up to a minute for the gateway to release a just-deleted sandbox's reference); under--keep-sandboxthe credential is expired in place instead.auth.jsonthroughsandbox execand verifies the file names the new generation (one repeat covers an iteration seed racing it). A settle timeout fails the attempt rather than re-seeding the old placeholder. A static key has its expiry pushed out and is re-seeded the same way, because an expiry-only update is a new generation too and the generation pi holds keeps the expiry it was built with. Bounded retries, then the run fails visibly; stopped before cleanup.config.yamlidentifiers (internal/config,fullsend github setup --openai-audience --openai-identity-provider-id --openai-service-account-id) — the three ids are recorded in the committedinference.openaiblock the same way the Vertex project and provider are (all-three-or-none flags; each id layers independently). They are identifiers, not secrets: a token is issued only to a caller whose GitHub OIDC claims match the mapping, and pull-request events read the config from the base branch (ADR 0033). Withinconfig.yamlthe three layer field by field (a base preset can carry the org's audience and provider id, each repository its service account). TheFULLSEND_OPENAI_*repository variables remain the opt-out and win when set; on a machine without a GitHub OIDC endpoint a setOPENAI_API_KEYwins over the committed block, so local runs of a repository that committed it still work.env:on everyfullsend runstep (thirteen steps, pinned byTestOpenAIVariableForwarding);providers/is now in everyLAYERED_DIRSlist (pre-existing docs(#5670): add fixture authoring section to behaviour-testing guide #5673 gap),profiles/deliberately not (TestLayeredDirsMatchWorkspacePreparation).features/runtime/pi-openai.feature(@requires:capability:runtime-pi-openai, carriespolicy: policies/base.yaml), not declared in the Makefile until an OpenAI organization is mapped to the pool repositories.docs/guides/infrastructure/openai-workload-identity.md(console steps, claims check, mapping onrepository+ref— fullsend runs agents from seven workflow files, so a singleworkflow_refassertion cannot cover them — incl. which runs it trusts,fullsend github setup --openai-*or variables, model selection, local runs incl. the policy requirement, troubleshooting),docs/runtimes/pi.md, running-agents-locally,docs/cli/run.md/github.md, layered-config-reference, contributing/architecture cross-references.Verified against OpenShell 0.0.115 (2026-08-27)
main bumped the pin from 0.0.83 to 0.0.115 while this PR was in review; every OpenShell behaviour the design relies on was re-verified against the 0.0.115 source and a live local gateway, and two of them had changed:
openshell:resolve:env, a colon, thenv<opaque>_OPENAI_API_KEY) and resolves (200); the canonical, unrevisioned form (OPENAI_API_KEYin that namespace) returns500 credential_unavailablefor this endpoint-bound credential (secrets.rs resolve_placeholder, "provider-shaped aliases carry no credential identity"); afterprovider updateto an invalid value the old placeholder still returned 200 with the old token and the new placeholder 401 — and still after eleven further updates — while a freshsandbox exechad the new placeholder within ~20 s; expiring the credential in place made every generation fail closed. This retires the round-2 "current alias" hand-off and is why pi now reads the credential from a runner-seededauth.jsonthat is re-seeded after each refresh. It also contradicts the 0.0.115providers-v2docs ("the proxy resolves existing placeholders against current credentials … without restarting the process"), which describe gateway-managed refresh handles, notstatic/externalcredentials; recorded in the ADR.policy.rs find_uninspected_credentialed_endpoint, supervisor: "credentialed endpoint requires L7 inspection; raw tunnel is not explicitly allowed"). A harness withoutpolicy:inherits the base image's default policy, whosecodexrule allowsapi.openai.com:443as an L4 tunnel for/usr/bin/node, and pi got connection errors; making that entryprotocol: restin the live sandbox gave 200, and the fleet'spolicies/base.yaml(no network rules) works as-is. Hence the egress preflight and the policy requirement in the docs.AuthStoragere-readingauth.jsonon a file-revision change andprepareRequestresolving auth per request — is verified in the pi 0.84.3 source (auth-storage.ts,packages/ai/src/auth/resolve.ts,model-runtime.ts;auth.jsonoutranks the environment and only--api-keyoutranks it) and exercised across iteration starts locally; a rotation inside one running iteration has not been observed live (two attempts to drive pi's RPC mode inside a bare sandbox stalled before its first request) — the first WIF run is that check, and a failure would be visible as a 401 after the old token's expiry. Recorded in the ADR.openshell:resolve:env+:), treating it as credential-bearing traffic — reproduced on the local gateway with a plainPOST /v1/responsesbody, and it is why this PR's owndispatch / Reviewjob failed on every head (the review agent's request carried the diff; the sandbox log showsPOST request body credential traffic denied for aiplatform.googleapis.com).maincontains no such literal; this PR now builds the prefix from two parts in source and tests and never writes it contiguously in docs or in this description. Verified against the v0.0.115 source (substring guard insecrets.rs contains_raw_reserved_marker, applied to every inspected REST endpoint once the sandbox holds any static credential — hence credential-less Vertex being hit): intended fail-closed design, with the bare-prefix false positive tracked upstream as bug(supervisor): literal and foreign-provider placeholders in conversation history block inference NVIDIA/OpenShell#2904; the only opt-out isallow_uninspected_credentials: trueon the model endpoint, adopted for the two default model profiles (fullsend-openai,fullsend-vertex-ai; verified: a body carrying the prefix gets 200 while the bearer header is still injected andGET /v1/modelsstays 403). The fleet's own agents resolve their Vertex profile from fullsend-ai/agents, which needs the same one-line change to unblock fullsend's review agent on such content — tracked in OpenShell 0.0.110+ resets model requests whose body mentions the placeholder prefix; fullsend agents cannot read text containing it #6716.--credential KEY=accepted),provider update --credential-expires-at KEY=RFC3339,! Provider <name> not foundon delete,FAILED_PRECONDITIONdelete while attached,list-profiles -o json(array withid), static credentials endpoint-bound (which closes the ADR-0025 residual this PR inherited).Static-key path end to end on this branch against the local 0.0.115 gateway with
fullsend-sandbox:latest(pi 0.84.3) andopenai/gpt-5.6-luna, customproviders: [openai]+policy: policies/base.yamlharness: embedded profile imported, run-scoped provider created empty then stored with its expiry, sandbox attached, egress preflight passed, pi read the placeholder from the seededauth.json,POST /v1/responsesallowed through the inspected provider entry, tool call through the hook adapter,agent-result.jsonwritten, tokens counted, exit 0; provider deleted at the end (expired in place under--keep-sandbox). The WIF path awaits an OpenAI organization.Testing
go test ./internal/inference/openaiwif/— exchange happy path, JSON body, audience encoding, non-200s, oversized/non-JSON bodies, timeout, missing config, non-Bearer rejected, lifetime cap, whitespace, OIDC host, secrets never in errors.go test ./internal/cli/— credential resolution (variables, config block, partial sources, static), run-scoped naming,ensureOpenAIProvideragainst a recordingopenshellstub, reserved profile id, embedded definition fallback (OpenAI only), egress preflight (uninspectedEndpointRules, host globs, unreadable policy = warning), re-seed after refresh (polls until the placeholder changes, seeds once), cleanup (delete / detach-race retry / expire-in-place), refresh loop,runAgent-level provider lifecycle with and without--keep-sandbox,github setupflags (all-or-none, overlay).go test ./internal/config/—inference.openailayering, setter round-trip, zero value removes the block.go test ./internal/runtime/— OpenAI gate;PiOpenAIAuthSeedunder a realsh(shape, atomic rename, refusals,.envcannot reach the file); the config-dir guard (emptyauth.jsonpasses, seeded entry passes pretty-printed or not, real key / other provider / extra entry / escaped key fail,models.jsonfails, shadowed builtins defeated byunset -f); no--api-key; ordering around.env.go test ./internal/sandbox/— literal create/update args,DenyExpansionKeys, expiry, delete,profileListed(JSON and table),StripPolicyHeader.go test ./internal/scaffold/— variable forwarding on every run step, layered dirs.make lintclean.Out of scope
fullsend repos installsupport for OpenAI (ValidProviders()staysvertex-only).OPENAI_API_KEY.Closes #6689