Repository navigation
Add isolated OIDC admission and runtime credit allowances - #70429
Conversation
Condition admission and OIDC acquisition on managed endpoint configuration. Valid approval supersedes daily validation; disabled admission and ignored outages retain daily fallback. Keep required primary and optional component allowances inside activation, use caller-owned traceparent correlation, and leave agent jobs unchanged. Align the specification, regression coverage, and TLA+ model. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Initialize the actions/github-script globals before loading the admission client. Include the compiler-owned frontmatter/body fingerprint in requests as a stable workflow_hash cache hint, with strict validation and documented authorization and cache-isolation requirements. Regenerate all workflow locks without changing downstream jobs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject the unsupported admission-gate frontmatter switch, retain admission in every activation job, and grant OIDC to hand-authored reusable callers. Assemble admission module paths with node:path before initializing github-script globals and loading the client. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve admission deadlines, cancellation, retry classification, and sanitized diagnostics while delegating retry policy to withRetry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add optional operation timeouts, monotonic deadlines, and cancellable waits to the shared retry helper. Replace admission's bespoke timer and abort handling while preserving its retry budget and sanitized errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Commit pushed:
|
There was a problem hiding this comment.
Request changes
Two blocking issues found in the admission runtime:
- The control-character regex rejects ordinary uppercase characters, so valid configured endpoints and identities fail admission.
id-token: writeis granted unconditionally to every activation job, including workflows with admission disabled; the step condition cannot sandbox OIDC minting.
Inline comments explain the impact and surgical fixes. Static review only; no tests or repository code were executed.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 6.46 AIC · ⌖ 0.553 AIC · ⊞ 1.6K
Comment /review to run again
🏗️ Design Decision Gate — ADR RequiredThis PR adds about 327 new lines of core business logic (more than the 100-line threshold) but does not link an Architecture Decision Record (ADR). 📄 Draft ADR committed: The draft records these decisions:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify that the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
Apply validated admission limits to agent, threat-detection, and evals runtime credit budgets, preserving existing policy for absent optional allowances and skipped or ignored admission. Add privacy-preserving lifecycle logs and collapsed step summaries, extend runtime JSON regression coverage, update the specification and TLA+ invariants, and regenerate workflows. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the concurrently added draft ADR and update its decision and consequences for the requested runtime budgets and redacted reporting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd, /codebase-design, and /grill-with-docs on the admission-runtime high-impact files (compiler_admission.go, check_admission.cjs, admission_protocol.cjs, compiler_activation_permissions.go, call_workflow_permissions.go, error_recovery.cjs, and the updated spec). Overall this is a well-scoped, defense-in-depth implementation with strong test coverage (128 JS tests, dedicated Go compiler tests, and a TLA+ model with 27 negative controls). Approving with a few minor, non-blocking suggestions.
📋 Key Themes & Highlights
Key Themes
- Strict protocol parsing: admission_protocol.cjs uses a hand-rolled regex grammar instead of trusting JSON.parse output directly, correctly handling duplicate-key and escaped-key attacks that a naive parse would miss — good defensive design for an untrusted external response.
- Deadline/signal plumbing: error_recovery.cjs's new runWithinRetryBudget plus AbortSignal wiring through withRetry is a clean, backward-compatible extension (existing non-timeout callers are unaffected since operation() and sleep() still work without a signal).
- Permission minimality trade-off documented: the PR body explicitly and correctly caveats that id-token: write on activation is not a sandbox boundary — good, since that is exactly the kind of assumption a reviewer would otherwise flag as a gap.
- Two inline comments note minor coverage/documentation gaps: IPv6-without-brackets URL validation test, and the non-overridable id-token: write behavior in extractPermissionsFromMDFile deserving an explicit code comment since it diverges from the otherwise-declarative frontmatter permissions: convention elsewhere in the compiler.
- One inline suggestion on runWithinRetryBudget's already-expired-deadline path is largely already covered by an equivalent withRetry-level test ("should not invoke operations after an expired deadline or zero timeout"), so treat it as a nice-to-have, not a gap.
Positive Highlights
- ✅ Spec, TLA+ model, and implementation were updated together and stay internally consistent (verified hash pattern, trace fields, and optional-allowance semantics all match across .tla, .md, and .cjs/.go).
- ✅ TestAdmissionRepositoryReusableCallerPermissions and TestAdmissionReusableCallerPermissionsWithStaleWorkerLock proactively guard the cross-cutting 344-lock-file regeneration against silent permission regressions.
- ✅ Bounded body reads (readBoundedBody), strict content-type checks, and redirect: "manual" on both OIDC and admission fetches are solid baseline hardening against response-smuggling/SSRF-adjacent issues.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 111.1 AIC · ⌖ 15.3 AIC · ⊞ 10.1K
Comment /matt to run again
There was a problem hiding this comment.
🔵 Needs a closer look
Admission output and permission regressions, stale WASM fixtures, and missing caller-migration guidance block approval.
No open issues 🎉
What changed in this PR
Adds activation-only OIDC admission, intended to replace daily-AIC validation after approval without changing downstream execution or runtime budgets.
Changes:
- Generates endpoint-gated admission checks with workflow hashes.
- Expands activation and reusable-caller OIDC permissions.
- Updates generated workflows, tracing, tests, documentation, and formal checks.
Unresolved issues include overwritten admission output, Markdown-worker write-all permission downgrades, stale WASM golden fixtures, and missing breaking-change migration guidance. Optional findings concern sequential OTLP export delays, duplicated action-path handling, and undocumented constants.
| File | Description |
|---|---|
specs/workflow-security/check.py |
Updates admission controls and reachability witnesses. |
specs/workflow-security/ActivationAdmission.cfg |
Selects revised admission invariants. |
pkg/workflow/compilerenv/manager.go |
Defines managed admission variables. |
pkg/workflow/compiler_admission.go |
Generates the conditional admission step. |
pkg/workflow/compiler_activation_permissions.go |
Grants activation OIDC access unconditionally. |
pkg/workflow/compiler_activation_job.go |
Inserts admission before validation. |
pkg/workflow/call_workflow_permissions.go |
Extends reusable-caller permission inference. |
pkg/workflow/call_workflow_permissions_test.go |
Updates permission-import expectations. |
pkg/workflow/agy_engine_test.go |
Updates scoped OIDC permission assertions. |
pkg/cli/env_admission_test.go |
Tests admission defaults and validation. |
docs/src/content/docs/reference/enterprise-configuration.md |
Documents centralized admission deployment. |
docs/src/content/docs/reference/cost-management.md |
Explains admission precedence and daily-AIC fallback. |
docs/src/content/docs/reference/compiler-enterprise-environment-controls.md |
Documents managed variables and permission boundaries. |
actions/setup/js/send_otlp_span.test.cjs |
Tests rejection of zero trace identifiers. |
actions/setup/js/send_otlp_span.cjs |
Adds validated W3C traceparent construction. |
actions/setup/js/error_codes.cjs |
Defines the admission error category. |
.github/workflows/windows.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/windows-grower.md |
Grants reusable-caller OIDC access. |
.github/workflows/unbloat-docs.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/tidy.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/test-quality-sentinel.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/squad.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/squad-plan.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-water.yml |
Grants reusable-caller OIDC access. |
.github/workflows/smoke-trigger.yml |
Grants reusable-caller OIDC access. |
.github/workflows/smoke-test-tools.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-temporary-id.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-service-ports.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-project.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-pi.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-pi-routed.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-opencode.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-multi-pr.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-multi-caller.yml |
Grants reusable-caller OIDC access. |
.github/workflows/smoke-kiro.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-goose.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-gemini.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-drive.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-cursor.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-crush.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-copilot.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-copilot-small.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-copilot-sdk.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-copilot-auto.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-copilot-arm.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-codex.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-claude.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-checkout-pr-dispatch.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-aider.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-agy.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-agent-public-none.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-agent-public-approved.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/smoke-agent-all-none.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/skillet.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/security-review.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/scout.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/refiner.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/q.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/purelock.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/pr-sous-chef.lock.yml |
Adds activation admission and OIDC access. |
.github/workflows/pr-nitpick-reviewer.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/ponytail-reviewer.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/plan.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/pdf-summary.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/mergefest.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/grumpy-reviewer.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/design-decision-gate.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/dependabot-burner.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/deep-report.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/daily-max-ai-credits-test.lock.yml |
Adds activation admission and OIDC access. |
.github/workflows/daily-issues-report.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/daily-firewall-report.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/daily-doc-updater.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/daily-doc-healer.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/daily-community-attribution.lock.yml |
Gates wiki-ledger preparation and daily validation. |
.github/workflows/daily-code-metrics.lock.yml |
Gates repository-ledger preparation and daily validation. |
.github/workflows/daily-cli-performance.lock.yml |
Gates repository-ledger preparation and daily validation. |
.github/workflows/credentials-check.yml |
Grants reusable-caller OIDC access. |
.github/workflows/craft.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/code-simplifier.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/cloclo.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/ci-coach.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/changeset.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/avenger.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/artifacts-summary.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/archie.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/approach-validator.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/ai-moderator.lock.yml |
Adds admission and daily-AIC gating. |
.github/workflows/ace-editor.lock.yml |
Adds admission after strict-policy enforcement. |
.changeset/admission-service-runtime.md |
Announces admission runtime behavior and boundaries. |
Review effort: Max
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Regenerate workflow locks and verify the needs-consistency linter reports no findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ Smoke OTEL completed successfully!
|
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟
|
|
✨ Gemini awakens... Smoke Gemini begins its journey on this pull request... |
|
🥧 Pi initializing... Smoke Pi begins on this pull request... |
|
✅ Smoke Copilot Auto completed successfully!
|
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready.
|
|
Gate asks for a token, Inspired by PR #70429's OIDC admission job and runtime credit allowances.
|
|
Smoke Test: MAI-Code-1.1-Flash
|
Comment MemoryPeek at saved memory (smoke-codex)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
|
Commit pushed:
|
Agent Container Tool Check
Result: 12/12 tools available ✅ Playwright title validation
|
|
Smoke Copilot: FAIL (partial) — PR "Add isolated OIDC admission and runtime credit allowances" by
|
Why
Implement the OIDC admission contract introduced in #70286 so participating workflows can consult a centralized authority before agent execution. Validated admission supersedes daily-AIC validation and supplies component-specific runtime credit limits.
Implementation
admissionjob, gated at job level byvars.GH_AW_DEFAULT_ADMISSION_URL. An absent endpoint skips its runner, setup and OIDC acquisition. Daily-AIC validation and its credentials remain inactivation; daily-limit variables do not enable admission.contents: readandid-token: write. Admission does not run workflow hooks or mint workflow/telemetry App credentials. Reject alljobs.admissionfrontmatter customization, includingsetup-steps,pre-steps,steps, permissions and environment overrides. Activation receives no admission-only OIDC grant; independent workflow OIDC requirements remain supported.guardrail.admissionunified-session events with admission provenance, and propagate the component values to detection/evals. Human-readable lifecycle logs and collapsed step-summary details withhold identities, credentials, endpoint configuration, allowance amounts, spend and service diagnostics.Validation
Operational scope
Admission is cooperative rather than an unbypassable organization spending cap. Authoritative enforcement still requires protected backend authorization/accounting. The independently implemented server contract is unchanged; its initial deployment profile supports GitHub.com, string audiences and non-reusable workflows, failing closed for unsupported reusable identities. No server or Actions rollout configuration is changed here.
Most of the diff is regenerated workflow lock files. Reusable caller permission envelopes must permit the callee's admission OIDC capability.
Changeset