Skip to content

Enforce detection gates for custom workflow jobs - #70997

Merged
pelikhan merged 7 commits into
mainfrom
pelikhan-ubiquitous-winner
Oct 22, 2026
Merged

pelikhan merged 7 commits into
mainfrom
pelikhan-ubiquitous-winner

Conversation

@pelikhan

@pelikhan pelikhan commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Custom jobs could consume tainted agent results before threat detection completed, or bypass failed detection with status-override conditions. These jobs must run before the agent or wait for successful, clean detection.

Changes

  • Extend custom-job-detection linting across Actions and AW ASTs, including transitive dependencies, unordered jobs, and jobs placed between agent and detection. Ordinary CI graphs without an agent are excluded; standalone locks recognize generated maintenance jobs.
  • Enforce the final compiled graph after dependency lowering. Downstream custom jobs, including reusable workflow calls, directly need detection and receive an outer guard requiring both successful detection job status and detection_success == 'true'. Authored conditions cannot bypass it; explicitly disabled detection remains exempt. Inactive reserved built-in names cannot be used to create custom jobs that evade the guard.
  • Share Actions expression parsing between compiler and lint, update CLI regression fixtures, and finalize ADR-70997.
  • Update compiler threat specification to 1.0.66, extending CTR-005 Unsafe Output Route with the custom-job contract, implementation/test mappings, and matching lint metadata. Update reference docs and regenerate workflow locks.

The clean-output check is intentional: warn-mode findings can leave the detection job successful while its output reports unsafe results.

ADR: ADR-70997: Enforce Detection Gates for Custom Workflow Jobs in the Compiled Job Graph

Verification

  • Merged the latest origin/main, preserving upstream prompt-output dependency checks alongside transitive detection gating.
  • Full AW, lint, and constants suites passed. Focused CLI input-mode, compiler dependency/detection, shared-parser, ownership, and specification-mapping checks passed.
  • All 346 workflows compile, and generated lock-file drift checks passed.
  • New-code golangci-lint reports 0 issues. The aggregate gate remains blocked by existing lint diagnostics in unchanged code within touched files and unknown-linter warnings.
pelikhan and others added 2 commits October 21, 2026 18:48
Lint transitive custom-job ordering in AW and Actions graphs and enforce successful clean detection before downstream custom jobs run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Update compiler threat specification to 1.0.66, document the ordering and clean-detection contract, and synchronize lint threat metadata and conformance mappings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review October 22, 2026 02:04
Copilot AI max review requested due to automatic review settings October 22, 2026 02:04
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Commit pushed: 2955ffe

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

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.

Generated from Ponytail Reviewer · gpt56 · 14 AIC · ⌖ 5.38 AIC · ⊞ 14.8K · ◷

Generated by Ponytail Reviewer for #70997

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

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.

Generated from Test Quality Sentinel · sonnet50 · 14.1 AIC · ⌖ 15.1 AIC · ◇ 6.61 AIC · ⊞ 12.7K · ◷

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@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.

Verdict

No actionable correctness, security, or performance defects were identified in the changed lines. The review was static; repository tests were not run per the review constraints. The advisory first-pass reviewer did not produce parseable findings, so this assessment is based on the independent diff analysis.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 2.16 AIC · ⌖ 0.54 AIC · ⊞ 1.6K
Comment /review to run again

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Design Decision Gate — ADR Required

This PR adds 458 new lines of core business logic under pkg/, which is over the 100-line threshold, but it does not link an Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/70997-enforce-detection-gates-for-custom-workflow-jobs.md. Review and complete it before merging.

🔒 This PR cannot merge until an ADR is linked in the PR body.

The draft records one decision. After dependency lowering, the compiler enforces detection gates on the compiled job graph in guardCustomJobDetection. Each downstream custom job gets a direct needs: detection and an outer guard that cannot be overridden: needs.detection.result == 'success' && needs.detection.outputs.detection_success == 'true'. Custom jobs that are unordered relative to agent, or that sit between agent and detection, are rejected. The custom-job-detection lint rule enforces the same contract, which is added to CTR-005.

📋 What to do next
  1. Review the draft ADR committed to your branch. It was generated from the PR diff.
  2. Complete the missing sections: add context the AI couldn't infer, refine the decision rationale, and list the alternatives you actually considered. The draft lists three: lint-only enforcement, a guard on job status only, and enforcement on the authored graph.
  3. Check the negative consequences in the draft: authored if conditions are rewritten and cannot opt out, post-agent jobs can no longer run when detection fails, and some existing graphs no longer compile.
  4. Commit the finalized ADR to docs/adr/ on your branch.
  5. Reference the ADR in this PR body by adding a line such as:

    ADR: ADR-70997: Enforce Detection Gates for Custom Workflow Jobs in the Compiled Job Graph

Once an ADR is linked in the PR body, this gate will re-run and check that the implementation matches the decision.

❓ Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

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 Reference

An ADR must contain these four sections to be considered complete:

  • Context: What is the problem? What forces are at play?
  • Decision: What did you decide? Why?
  • Alternatives Considered: What else could have been done?
  • Consequences: What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus55 · 31 AIC · ⌖ 40.7 AIC · ⊞ 1.9K · ◷
Comment /review to run again

Copilot AI 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.

🟡 Changes recommended

An authored reusable job can still bypass detection by using an inactive built-in name.

4 open issues
What changed in this PR

This PR strengthens gh-aw’s compiler and lint checks so custom jobs run before the agent or wait for successful, clean threat detection.

Changes:

  • Adds transitive dependency checks and compiler-enforced detection guards, including for reusable workflow calls.
  • Adds regression tests and documents the CTR-005 contract in specification version 1.0.66.
  • Regenerates workflow locks with clean-detection guards and updated release dependencies.
File Description
specs/​compiler-threat-detection-spec.md Defines custom-job ordering and gating requirements.
specs/​compiler-threat-detection-changelog.md Records specification version 1.0.66.
pkg/​workflow/​lint/​rule_contract_test.go Updates rule metadata and Actions contracts.
pkg/​workflow/​lint/​custom_job_detection.go Extends detection linting across dependency graphs.
pkg/​workflow/​lint/​custom_job_detection_test.go Adds transitive and Actions lint cases.
pkg/​workflow/​compiler_yaml.go Invokes detection enforcement after dependency lowering.
pkg/​workflow/​compiler_main_job_helpers.go Uses transitive dependencies for agent scheduling.
pkg/​workflow/​compiler_main_job_helpers_test.go Covers combined activation dependencies.
pkg/​workflow/​compiler_custom_job_detection.go Adds final-graph validation and detection guards.
pkg/​workflow/​compiler_custom_job_detection_test.go Covers compiler ordering, guards, and exemptions.
pkg/​workflow/​aw/​job_dependencies.go Introduces shared dependency-graph traversal.
pkg/​workflow/​aw/​job_dependencies_test.go Tests transitive and cycle-safe traversal.
docs/​src/​content/​docs/​reference/​workflow-linters.md Explains expanded lint and compiler enforcement.
docs/​src/​content/​docs/​reference/​steps-jobs.md Documents downstream custom-job requirements.
.github/​workflows/​smoke-work-queue.lock.yml Gates result verification on clean detection.
.github/​workflows/​smoke-test-tools.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-temporary-id.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-service-ports.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-project.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-opencode.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-kiro.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-goose.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-gemini.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-deepseek-harness.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-cursor.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-crush.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-copilot.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-copilot-arm.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-copilot-aoai-entra.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-copilot-aoai-apikey.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​smoke-codex.lock.yml Gates telemetry, secret scanning, and reporting.
.github/​workflows/​smoke-claude.lock.yml Adds clean-detection telemetry gating.
.github/​workflows/​release.lock.yml Expands agent prerequisites and success checks.
.github/​workflows/​github-remote-mcp-auth-test.lock.yml Gates the MCP canary on clean detection.
.github/​workflows/​firewall-escape.lock.yml Gates failure reporting on clean detection.
.github/​workflows/​daily-max-ai-credits-test.lock.yml Gates credit verification on clean detection.

Review effort: Max

@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.

Skills-Based Review 🧠

Applied /codebase-design and /improve-codebase-architecture (per pr-triage: architecture_change). The new transitive JobDependencyGraph abstraction and guardCustomJobDetection consistently mirror the existing guardGatewayProvenanceJobs pattern (inject needs + wrap if with a non-overridable guard + actionsCondition = true), and test coverage in compiler_custom_job_detection_test.go / lint/custom_job_detection_test.go is thorough (ordering, transitive paths, bypass conditions, malformed if, disabled detection). No blocking issues found — posting a few targeted improvement suggestions.

📋 Key Themes & Highlights

Key Themes

  • Minor consistency nit: AuthoredJobDependencies's built-in exclusion switch uses raw string literals instead of the already-imported constants.*JobName typed constants, risking silent drift from KnownBuiltInJobNames.
  • Duplicated validation logic: the actionlint lexer/parser expression-validation in addCustomJobDetectionGuard (compiler) closely duplicates customJobDetectionCondition (lint). Worth extracting into one shared helper (e.g. in pkg/workflow/aw) so the "one valid Actions expression" contract is defined once.
  • Error message drift: the compiler and lint diagnostics for the same "must run before agent or after detection" violation use slightly different wording, which could read as two different rules to a user hitting both.

Positive Highlights

  • ✅ Dependency-graph abstraction (JobDependencyGraph.DependsOn) is cycle-safe and cleanly separates graph construction from traversal.
  • ✅ Strong parity between compiler enforcement and lint detection (dual-AST support, transitive checks, ordering diagnostics) — docs in workflow-linters.md and steps-jobs.md are updated to match the new semantics.
  • ✅ Test coverage exercises the tricky cases: jobs sitting between agent/detection, pre-agent jobs, warn-mode bypass via always()/||, and malformed if expressions.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 171.3 AIC · ⌖ 16.3 AIC · ⊞ 10.2K
Comment /matt to run again

Comment thread pkg/workflow/aw/job_dependencies.go Resolved
@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_custom_job_detection.go:26): Exempt only jobs that were actually compiler-generated. With detection enabled but no top-level evals configuration, jobs.evals: {needs: agent, uses: ./.github/workflows/report.yml} is still created by buildCustomJobs: shouldSkipCustomJob skips existing jobs, not all reserved IDs, and reusable calls avoid the built-in step restrictions. This branch then skips detection enforcement solely because of the name. The authored call receives neither a detection dependency nor the clean-output guard, so it can consume agent results before detection finishes. Other inactive optional built-in names have the same gap. Reject reserved-name custom-job creation or track generated-job provenance while preserving supported built-in customizations, and add a compilation regression for this reusable-call case. - Enforce detection gates for custom workflow jobs #70997 (comment)
  3. Review (pkg/workflow/lint/custom_job_detection.go:21): Update TestCustomJobDetectionCLIInputModes in pkg/cli/lint_workflow_test.go:345–375 alongside this contract change. Its .md case still expects an empty ThreatRuleID, but the runner now attaches CTR-005. Its .lock.yml case still expects a jobs.report.if error even though the compiled fixture contains only detection; the new authoritative-graph path finds no report job and returns no diagnostics. These assertions now fail. Expect CTR-005, and either include agent and an unsafe report job in the compiled fixture or explicitly expect no diagnostics for the detection-only graph. - Enforce detection gates for custom workflow jobs #70997 (comment)
  4. Review (pkg/workflow/lint/custom_job_detection.go:36): Skip Actions graphs without an agent job before applying the detection policy. The new Actions target also runs on ordinary CI workflows, where detection may mean path-change detection rather than threat detection. For example, notify with needs: detection and if: always() now receives an error about bypassing threat detection even when no agent exists. The hasAgent check only protects the unordered-job branch, so downstream jobs still trigger false errors. Require an agent job for Actions analysis and add a native-workflow regression. - Enforce detection gates for custom workflow jobs #70997 (comment)
  5. Review (pkg/workflow/lint/custom_job_detection.go:119): Without a companion AW document, this path treats every job outside KnownBuiltInJobNames as custom, but that set omits compiler-generated state-push jobs. For example, daily-mcp-concurrency-analysis.lock.yml:2868–2873 generates push_evals_state without a direct detection dependency, and lines 2955–2962 generate push_ledger_changes with if: always(). Both now produce error-severity findings when the lock is linted without its source. Generated push_experiments_state jobs are also reported as unordered. Correct compiled workflows should not fail this rule on compiler-owned jobs. Use a complete, shared ownership policy for Actions-only classification and cover standalone lock files, while keeping authored custom jobs subject to detection enforcement. - Enforce detection gates for custom workflow jobs #70997 (comment)
  6. Review (pkg/workflow/aw/job_dependencies.go:34): [/codebase-design] The built-in exclusion switch uses raw string literals ("pre_activation", "activation", "admission", "agent") instead of the typed constants.*JobName constants already imported in this file (e.g. constants.PreActivationJobName, constants.ActivationJobName, constants.AdmissionJobName, constants.AgentJobName). If a built-in job name ever changes, this switch can silently drift out of sync with KnownBuiltInJobNames while still compiling. - Enforce detection gates for custom workflow jobs #70997 (comment)
  7. Review (pkg/workflow/compiler_custom_job_detection.go:52): [/codebase-design] This actionlint lexer/parser offset-check for validating a job if condition duplicates the parsing logic in pkg/workflow/lint/custom_job_detection.go's customJobDetectionCondition (lines ~213-233 there). Two near-identical actionlint-based validators now live in different packages (workflow vs workflow/lint) and can drift apart over time (e.g. the lint version strips ${{ }} wrapper differently before parsing). - Enforce detection gates for custom workflow jobs #70997 (comment)
  8. Review (pkg/workflow/compiler_custom_job_detection.go:34): [/grill-with-docs] This error wording ("add this job to jobs.agent.needs or add detection to this job's needs") differs from the parallel lint diagnostic in lint/custom_job_detection.go line 54 ("make agent depend on this job or add detection to this job's needs") for the same violation. Minor, but since both paths are now documented together in workflow-linters.md, aligning the phrasing would reduce confusion when a user sees one message from gh aw compile and a differently-worded one from gh aw lint. - Enforce detection gates for custom workflow jobs #70997 (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: 2955ffe
Sous-chef work: 00f20767aa545e7e7b5e42aefee44496fbec7cc4f89583adb7669e7094713582 02a099868949d6612bfa18d37938f9ff4f95fc68363b315e52746d12646d54a7 2d76a1bab730c4a045351ddb907186c4c12cca43b558070039b96c22f8e89ba4 444de82d2ad37e423756e456cb55d45dbb62ded276ffdc2f14e00e76b6b9eb3c 51fef73265fdab4255ddf377ec6622b9a45d4a04d9ea697a46e8066f7c80483c 56e13769f50339ac6154bea8d43ba2a3575830a3a8998edba199be46a96a61c8 a9efe751df954428a2bbf0f3e8bf2952d6e99d9e022e3ae61b2e9f4b92b08a3a
Sous-chef state: f6ee856aff28530e6ec7200320d6a3fa0f290d1c3c3f41ede9da17aa55c01da8

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

pelikhan and others added 2 commits October 21, 2026 20:31
Preserve transitive detection gating while adopting upstream prompt-output dependency checks and regenerate compiled workflows from merged sources.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject inactive reserved-name jobs, distinguish generated maintenance jobs in standalone linting, skip non-agent Actions graphs, share expression parsing, fix CLI fixtures, and finalize ADR-70997.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): Accepted the ADR follow-up. The generated draft is now finalized and linked from the PR body.

@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): No change is needed for this completion notice because it contains no actionable finding. Focused regressions and the full AST suites were run independently for the review fixes.

@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): No additional change is needed for the no-findings review summary. The separate correctness findings were fixed and covered by compiler, lint, and CLI regressions.

@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): No change is needed for the no-action test-quality notice. Added and ran regressions for reserved-name calls, generated maintenance jobs, non-agent workflows, shared expression parsing, and CLI input modes.

@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): Accepted the associated skills-review suggestions. Typed job-name constants, shared expression parsing, and aligned diagnostics are implemented, and each inline thread has been answered.

@pelikhan

Copy link
Copy Markdown
Collaborator Author

In response to #70997 (comment): Accepted. ADR-70997 is finalized with context, rationale, alternatives, and negative consequences, committed in 22b06ce, and linked from the PR body.

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @pelikhan! This PR touches 244 files (5,470 lines), mostly lock regenerations. Consider splitting core detection-gate logic and tests from spec updates and lock-file regeneration for easier review.

Generated by ✅ Contribution Check · copilot · auto · 31.7 AIC · ⌖ 0.645 AIC · ⊞ 8.7K · ◷

Integrate model-routing and detection alias updates, regenerate workflows, and remove an undefined redundant postAgentJobs check introduced by a recent branch commit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.93.1

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

Labels

None yet

4 participants