Skip to content

Commit 31afa0e

Browse files
bradygasterCopilot
andauthored
fix(gh-aw): enforce deterministic command authorization (#2187)
Closes #2185 Enforce trusted actor/author authorization, constrained open-mode outputs, and guarded custom/conclusion paths. Includes independent security revisions, pinned-runtime regression coverage, regenerated workflow package integrity, and resolved Copilot review findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83321e5d-2358-40a2-9307-bf37b077165b
1 parent 9815d63 commit 31afa0e

17 files changed

Lines changed: 1111 additions & 243 deletions

‎docs/src/content/docs/guide/gh-aw.md‎

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,25 @@ PR conversation comment. On a pull request, post the command in the
741741
Commands are matched longest-prefix-first, so the most specific command string
742742
wins: `/squad plan accept scope` is not treated as `/squad plan`.
743743
744+
### Command diagnostics
745+
746+
The dispatcher and command router disable automatic failure, failed-job,
747+
detection and no-op tracking-issue reporting, plus generic activation/completion
748+
comments. Those framework paths run outside the deterministic safe-output guard;
749+
completion comments can also append raw agent no-op text even when that guard
750+
rejects the output. This restriction applies to maintainer runs as well as open
751+
modes. Guarded command replies, research/planning artifacts and Cast failure
752+
comments remain supported.
753+
754+
Use the Actions run's logs and summaries for framework diagnostics. No-op messages
755+
remain visible there, `report_incomplete` fails the conclusion step, and
756+
`missing_data` remains logged (it does not by itself guarantee a red run).
757+
Threat detection still runs and retains its enforcement behavior; only its
758+
tracking-issue reporting is disabled. No diagnostic is permission to bypass
759+
command authorization or the open-mode artifact restrictions.
760+
761+
### Command reference
762+
744763
| Category | Command | Purpose | Notes |
745764
|----------|---------|---------|-------|
746765
| Team | `/squad` | Cast a new team | Same as `/squad cast` |
@@ -845,6 +864,41 @@ together. Direct manual dispatches of either worker fail closed; use
845864
`/squad implement` or the retrospective relay so the dispatcher can create the
846865
authoritative receipt and bound session inputs.
847866
867+
### Deterministic command authorization
868+
869+
Direct slash commands and discovered commands share a trusted runtime check,
870+
independent of agent instructions. Mutating commands require live **write,
871+
maintain, or admin** repository permission for both the event actor and the
872+
original issue/comment author. Missing identities, bot-authored text, failed
873+
permission lookups, and edited events without changed-command evidence fail
874+
closed. Editing a maintainer's old comment does not inherit their authority.
875+
876+
`status`, `review`, `research`, and `plan` remain open to identifiable human
877+
contributors without write permission. Their safe outputs are restricted to
878+
comments on the originating issue and mode-specific draft artifacts. They
879+
cannot create issues or PRs, change labels, dispatch workers, or write acceptance
880+
or activation artifacts, even when the caller is a maintainer. Research and
881+
plan can update only their own nonterminal lifecycle state. Put structured data
882+
in the tool's `data` field, not JSON fences or `Structured data:` blocks in the
883+
comment body. This restriction includes HTML comments and quoted/code regions:
884+
gh-aw preserves HTML comments inside code fences, and lifecycle readers inspect
885+
the complete posted text rather than its rendered visibility.
886+
887+
The guard validates the entire emitted batch before the built-in handlers run.
888+
Research/lifecycle custom jobs and terminal lifecycle repair require a successful
889+
safe-output job. The command router permits no agent-authored mutations; only
890+
its deterministic handler can forward a validated command.
891+
892+
Authenticated `workflow_dispatch` events retain GitHub's platform Actions-write
893+
authorization, including the router and implementation worker's `GITHUB_TOKEN`
894+
continuations. This exception is bound to the actual event and matching platform
895+
actor/sender, never an `aw_context` claim, issue text, or a blanket bot exemption.
896+
Open-mode output restrictions still apply to dispatched commands. Empty activation
897+
probes cannot produce safe outputs.
898+
899+
Install this protection as a complete immutable package update using
900+
[Upgrading](#upgrading), not by editing an integrity-pinned runtime file.
901+
848902
### Where you can use slash commands
849903
850904
| Surface | How it works |
@@ -1447,14 +1501,13 @@ interchangeable — the trigger, the wording, and the remedy differ:
14471501
| Issues not created | Created count is below the plan's declared total | `N of M issues created so far — rerun the identical activation command to continue.` |
14481502
| Labels not applied | An activated issue had no `add_labels` call accepted | `{labeled} of {activated} activated issues had a label operation accepted` |
14491503
1450-
Either one calls `report_incomplete`. **This does not fail the run.** The workflow
1451-
run still concludes `success`, so `gh run view --json conclusion` is not a way to
1452-
detect a truncated activation. The durable, user-visible signal is a tracking
1453-
issue in your repository titled `[aw] ... reported incomplete result`, which
1454-
gh-aw opens or updates:
1504+
Either one calls `report_incomplete`. With pinned gh-aw v0.89.22, **this fails the
1505+
conclusion step** and records the reason in the Actions logs. The dispatcher
1506+
does not create or update a separate tracking issue. Inspect the run and the
1507+
guarded activation summary on the originating issue:
14551508
14561509
```bash
1457-
gh issue list --search '"reported incomplete result" in:title' --state all
1510+
gh run view <run-id> --log-failed
14581511
```
14591512
14601513
If a cap was actually reached, the report names which cap and lists the work
@@ -1469,7 +1522,9 @@ IDs the run minted — `#aw_epic{K}` and `#aw_task{N}` on the hierarchical path,
14691522
— rather than by issue number, because at that point creation is still deferred
14701523
and no real number exists yet.
14711524
1472-
Do not read a green run as a complete activation. Check for that tracking issue.
1525+
Do not read a green run as proof of complete activation: an omitted
1526+
`report_incomplete` signal cannot be detected by that handler. Check the
1527+
activation summary and verify the resulting issues and labels.
14731528
14741529
#### Verifying an activation: the bindings block
14751530
@@ -2130,8 +2185,8 @@ other refs.
21302185
| `/squad` command is ignored | Lock file not committed or workflow not compiled | Run `gh aw compile --strict`, commit the lock file, and push |
21312186
| Universe is full on cast-member | All character names in the universe are allocated | Retire an unused member first, or re-cast with `/squad cast` |
21322187
| "No plan found" on plan accept | No `/squad plan` comment exists yet | Run `/squad plan` first to generate a plan for review |
2133-
| Plan activation creates fewer issues than the accepted plan declares | The run ended early, or it reached the `create-issue` (75) or `add-labels` (110) safe-output cap | Look for an `[aw] ... reported incomplete result` tracking issue — it names the shortfall and, when a cap was reached, which cap and what did not fit. Re-run the identical activation command (title matching resumes without duplicating existing issues), or activate one phase at a time with `/squad plan activate phase {N}` |
2134-
| Activation run is green but some issues are missing or unlabeled | `report_incomplete` records truncation without failing the run | A green run is not proof of a complete activation. Check for the `[aw] ... reported incomplete result` tracking issue, then verify with `gh issue list --label squad` |
2188+
| Plan activation creates fewer issues than the accepted plan declares | The run ended early, or it reached the `create-issue` (75) or `add-labels` (110) safe-output cap | Inspect the Actions logs and originating issue's activation summary for the shortfall and any cap reached. Re-run the identical activation command (title matching resumes without duplicating existing issues), or activate one phase at a time with `/squad plan activate phase {N}` |
2189+
| Activation run is green but some issues are missing or unlabeled | The agent may have omitted `report_incomplete`, which would fail the conclusion step if emitted | A green run is not proof of a complete activation. Check the activation summary and Actions logs, then verify with `gh issue list --label squad`; automatic tracking-issue reporting is disabled |
21352190
| A Squad label has no description and an unexpected color | It was auto-created on a fresh repo by `create-if-missing` | Expected, not a failure. Edit the label if you want a description or a specific color |
21362191
| `/squad implement` cannot create a PR | Expected under Profile A (recommended); Actions is not allowed to create pull requests and there is no automatic fallback for this workflow | Push the worker's branch and open the PR manually, or switch to [Profile B](#profile-b-opt-in-automatic-pr-creation) if you accept its repository-wide self-approval tradeoff |
21372192
| Epic implementation dispatches no workers | Every child is blocked or already has an open implementation PR | Merge dependency PRs, then run `/squad implement` on the epic again |

‎test/gh-aw-activation-capacity.test.ts‎

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
* that believed the label reading would conclude a 50-issue activation (up to 100 label
1515
* names) had overrun its budget, and could stop labeling early or batch issues together.
1616
* This is agent self-truncation, and it was reachable at 80.
17-
* 2. **Neither enforcement layer fails the run.** Even when a cap is genuinely hit, the
18-
* workflow run still concludes successfully, so nothing forces the truncation to
19-
* surface. See below.
17+
* 2. **Neither cap-enforcement layer fails the run by itself.** Even when a cap is
18+
* genuinely hit, truncation needs an explicit incomplete report to surface. See below.
2019
*
2120
* Note what is *not* claimed: at `max: 80`, 50 `add_labels` calls did **not** overflow the
2221
* operation cap. Runtime truncation was not reachable at the documented maximum. The cap
2322
* moves to 110 to defeat the misleading injected wording and to hold a bounded margin —
2423
* not to fix a proven 80-item overflow.
2524
*
26-
* ## Runtime semantics this suite is written against (gh-aw v0.87.10, the CI pin)
25+
* ## Runtime semantics this suite is written against (gh-aw v0.89.22, the CI pin)
2726
*
2827
* **Cap enforcement is dual (Safe Outputs Specification MCE4) and neither half is fatal.**
2928
* Invocation time — `safe_outputs_handlers.cjs`, `enforcePerTypeMax` via
@@ -38,20 +37,17 @@
3837
* That is a JSON-RPC error the agent **does** observe. Collection time —
3938
* `collect_ndjson_output.cjs` — is the second half: a surplus item is dropped with
4039
* `continue` and reported via `core.warning`. Neither path calls `core.setFailed`, so the
41-
* run concludes successfully with label operations missing.
40+
* run can conclude successfully with label operations missing unless incompletion is reported.
4241
*
4342
* `max` therefore caps **operations of that type**, not label names inside one call. One
4443
* `add_labels` call carrying two labels costs one unit of budget, not two.
4544
*
46-
* **`report_incomplete` does not turn the run red.** gh-aw's own tool description claims it
47-
* is "treated as a failure signal even when the agent exits successfully" — misleading in
48-
* precisely the way this issue is about. In the pinned runtime,
49-
* `report_incomplete_handler.cjs` emits `core.warning` only, and `handle_agent_failure.cjs`
50-
* contains no `core.setFailed` or `process.exit`: its "failure handling" opens or updates an
51-
* `[aw] {workflow} reported incomplete result` tracking issue/comment. That is a durable,
52-
* human-actionable record — it satisfies #1961's "explicit incomplete result" — but the run
53-
* conclusion stays green. No narrower supported mechanism in v0.87.10 makes it red, so the
54-
* workflow states that limitation rather than implying a failure it cannot produce.
45+
* **`report_incomplete` fails the conclusion step.** In v0.89.22,
46+
* `report_incomplete_handler.cjs` logs a warning, then `handle_agent_failure.cjs` calls
47+
* `core.setFailed` even when the agent succeeded. #2185 disables that conclusion handler's
48+
* independent tracking-issue writes, not its failure status or logged evidence. The guarded
49+
* activation summary must also report the shortfall on the originating issue. The real pinned
50+
* handler and zero-write boundary are exercised by gh-aw-command-authorization.test.ts.
5551
*
5652
* ## The capacity calculation this suite locks in
5753
*
@@ -412,33 +408,24 @@ describe('gh-aw: self-validation reconciles activated items with label operation
412408
).toBe(true);
413409
});
414410

415-
it('states report_incomplete semantics accurately — a tracking record, not a red run', () => {
416-
// Verified against pinned gh-aw v0.87.10 rather than gh-aw's own tool description
417-
// (which says "treated as a failure signal even when the agent exits successfully"
418-
// — misleading in exactly the way this issue is about):
419-
// report_incomplete_handler.cjs -> core.warning only.
420-
// handle_agent_failure.cjs -> contains no core.setFailed / process.exit;
421-
// "failure handling" opens/updates an
422-
// "[aw] ... reported incomplete result" issue.
423-
// The run still concludes successfully. The workflow must say so, so the agent does
424-
// not assume a red run is carrying the signal for it.
411+
it('describes pinned report_incomplete failure status without tracking-issue writes', () => {
412+
// v0.89.22 fails conclusion for report_incomplete. #2185 disables its independent
413+
// issue-writing path; the real-handler regression lives in command-authorization.
425414
expect(
426-
/reported incomplete result/i.test(activateProse),
427-
'The workflow must name the durable artifact report_incomplete actually produces.',
415+
/`report_incomplete` records the reason in Actions logs and fails the conclusion step/i.test(activateProse),
416+
'The workflow must describe the failure status and logged evidence from the pinned runtime.',
428417
).toBe(true);
429418
expect(
430-
/does \*\*not\*\* change the run's conclusion/i.test(activateProse),
431-
'The workflow must state that report_incomplete does not change the run conclusion.',
419+
/automatic tracking-issue reporting is disabled/i.test(activateProse),
420+
'The workflow must not promise an issue from the disabled conclusion reporting path.',
432421
).toBe(true);
433422
expect(
434-
/never rely on a red run/i.test(activateProse),
435-
'The workflow must forbid relying on a failed run to carry the incompletion signal.',
423+
/Include the shortfall in the guarded activation summary as well, so the originating issue retains an actionable signal/i.test(activateProse),
424+
'The authorized originating-issue summary must still identify incomplete activation.',
436425
).toBe(true);
437-
// Guard against reintroducing the overclaim this test previously asserted.
438426
expect(
439-
/failure signal even when the agent exits successfully/i.test(activateProse),
440-
'The workflow must not repeat gh-aw\'s misleading "failure signal" phrasing: the ' +
441-
'pinned runtime emits a warning and a tracking issue, and never fails the run.',
427+
/does \*\*not\*\* change the run's conclusion/i.test(activateProse),
428+
'The obsolete v0.87.10 success-conclusion claim must not return.',
442429
).toBe(false);
443430
});
444431

0 commit comments

Comments
 (0)