microsoft / microsoft/hve-core

feat(scripts): track caller-controlled provenance through matrix and job outputs

Open
#2,632 0 comments 1 reaction 1 assignee Claimed by @jkim323 View on GitHub
github-actions priority-3 scripts security workflows
Dominant language
Python
Stars
1.5k
Forks
301
Avg merge
3d 3h
Merged PRs (30d)
92

Description

## Issue Description

CQ-6 enforcement in `scripts/security/Test-DangerousWorkflow.ps1` matches `inputs.*` and `github.*` expressions directly. It does not follow a caller-controlled value once that value passes through `strategy.matrix`, a step output, or a job output. A caller-controlled input can therefore reach shell command text on the far side of the CQ-6 boundary without the gate reporting it.

This is the same class of gap that composite action coverage closed, one hop further out.

### Current state

A structured scan of `.github/workflows` produced the following inventory of `strategy.matrix` provenance:

| Matrix provenance | Jobs | `${{ matrix.* }}` references in `run:` bodies |
|-----------------------|-----:|---------------------------------------------:|
| Literal list | 3 | 3 |
| From `needs.*.outputs.*` | 14 | 24 |
| From `inputs.*` | 1 | 0 |
| **Total** | **18** | **27** |

There is **no current exposure**. The single job whose matrix derives directly from a caller input, `extension-marketplace-publish.publish` (`matrix: ${{ fromJson(inputs.packages-matrix) }}`), consumes `matrix.id` only in `env:` and artifact-name positions.

Every run-body matrix reference sits behind at least one `needs` hop and resolves to repository-controlled generation. The deepest chain is in `extension-package.yml`:

```
inputs.channel -> env INPUT_CHANNEL -> Get-MarketplacePackageMatrix.ps1
-> steps.discover.outputs.matrix -> needs.discover-packages.outputs.matrix
-> matrix.id in run bodies
```

### Design decision required first

Full provenance tracking would flag all 24 `needs`-derived references, every one of which is repository-controlled by inspection. Reporting them would force either 24 unnecessary migrations or a new suppression mechanism.

The CQ-6 implementation deliberately declined to add an inline suppression escape hatch, on the grounds that no case required one and it would widen the gate's surface. That decision has to be revisited before any detection code is written, because full taint tracking cannot land without either a sanitizer concept or an exception mechanism.

**The first deliverable is the sanitizer and exception design, not the graph walker.**

### Scope

* Decide whether the gate gains a sanitizer concept, an inline exception mechanism, or a provenance allowlist, and record the decision.
* Classify `strategy.matrix` and `matrix.include` provenance per job as literal, input-derived, or output-derived.
* Resolve `needs..outputs.` to the producing job's `outputs:` map, then to `steps..outputs.`, then to the step that wrote it. Reusable workflow calls mean this graph crosses files, so `on.workflow_call.outputs` must be parsed as well.
* Extend `scripts/security/Test-DangerousWorkflow.ps1` and its Pester coverage.
* Update the CQ-6 control note and the T-3 residual posture to reflect the new boundary.

### Acceptance Criteria

* [ ] The sanitizer or exception model is decided and documented before detection code is added.
* [ ] Matrix provenance is classified per job without relying on broad string matching.
* [ ] A `run:` or `actions/github-script` body consuming a matrix value derived from a caller-controlled input is reported with actionable file, job, and step context.
* [ ] Repository-controlled matrices produce no findings, and that outcome is asserted rather than assumed.
* [ ] `scripts/tests/security/Test-DangerousWorkflow.Tests.ps1` covers literal, input-derived, single-hop `needs`-derived, and cross-file reusable-workflow provenance.
* [ ] `docs/security/security-model.md` and `.github/workflows/README.md` reflect the final boundary and residual posture.
* [ ] `npm run test:ps -- -TestPath scripts/tests/security/Test-DangerousWorkflow.Tests.ps1` passes.
* [ ] `npm run lint:dangerous-workflow`, `npm run lint:yaml`, and `npm run validate:local` pass.

## Additional Context

Identified during review of the CQ-6 implementation for #2623, recorded there as finding F-3. That work established the `INPUT_` step-level `env:` pattern, the type-driven rule, and composite action coverage, all of which this issue builds on.

A cheaper partial option exists and may be worth splitting out: flag only matrices sourced **directly** from `inputs.*`, roughly 40 to 60 lines in the scanner plus about four fixtures, requiring no migrations. It would catch zero existing cases and serves purely as a forward guard, so it should be proposed on its own terms rather than presented as closing this gap.

Deliberately not labelled `agent-ready`. The blocking design decision on suppression semantics needs a human owner before implementation begins.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.