security: add weekly repository audit workflow - #95
Open
Gregory Joseph (gnjoseph) wants to merge 10 commits into
Open
security: add weekly repository audit workflow#95Gregory Joseph (gnjoseph) wants to merge 10 commits into
Gregory Joseph (gnjoseph) wants to merge 10 commits into
Conversation
Adds a scheduled (Monday) and manually dispatchable security audit workflow with deny-all default permissions, per-job least privilege, concurrency control, timeouts, and SHA-pinned actions. Deterministic jobs: CodeQL (security-extended) SARIF, npm audit reduced to sanitized counts, an effective Gitleaks CLI scan (pinned release plus SHA-256 checksum verification), and repo-wide action pin validation. The audit path never executes repository source or lifecycle scripts. The model-assisted job is implemented but intentionally inert: it is gated on a repository variable and a protected environment that do not exist yet, so it is skipped and the run summary reports "AI NOT_CONFIGURED" instead of claiming a pass. A synthetic dry-run job exercises the corpus, schema-validation, redaction, and SARIF conversion path with no credential, no network, and no code-scanning upload. Supporting zero-dependency Node ESM scripts live under scripts/security-audit/ (target/ref validation, corpus collection with hard file and byte caps, response schema validation and redaction, SARIF conversion, report sanitizers, pin checking, summary, dry run) together with fixtures and 39 node:test assertions covering trigger and permission invariants, SHA pinning, corpus caps, prompt injection, redaction, malformed and missing credential behaviour, absence of issue or comment creation, and dry-run SARIF output. Also fixes related repository governance: replaces the unresolvable CODEOWNERS owner with valid direct collaborators, replaces the no-op gitleaks job in security.yml that could report green without scanning, and SHA-pins the remaining CI actions. Validation: lint, typecheck, build, vitest (763 passed), audit test suite (39 passed), pin check (exit 0), dry run (exit 0), and workflow YAML parse gate all pass locally. AB#3219476 Co-authored-by: Copilot <copilot@github.com>
Addresses the post-implementation security review of the weekly repository
security audit workflow.
Blocking findings:
1. Scheduled runs passed an empty ref that validate-target rejected. An empty
or omitted ref now resolves to the origin/main tip via
`git rev-parse refs/remotes/origin/main^{commit}`, and the resolved value is
still held to the full 40-hex form and to reachability from main. Adds
regression coverage for the scheduled/empty-ref path, branch names, short
SHAs and unreachable SHAs.
2. Corpus delimiters were static and therefore forgeable; the sentinel already
occurs verbatim in lib/constants.mjs, which is itself inside the workflows
and full scopes. Delimiters are now derived from a 24-byte per-run
cryptographic nonce. Repository content that contains the static sentinel is
neutralized rather than rejected, so legitimate files remain auditable, and
collection aborts if any body contains the nonce. The nonce is carried in
the corpus manifest and injected into both rendered prompt files, and fence
integrity is asserted after collection and again before the prompt is built.
3. actions/ai-inference concatenates the system prompt with the user prompt, so
`system-prompt-file` is not a privileged role and cannot be relied on to
survive a hostile corpus. The immutable output, no-echo and schema contract
is now reasserted after the corpus as a trusted suffix
(scripts/security-audit/prompt-suffix.md), assembled by a new
build-prompt.mjs. Documentation states plainly that validate-response.mjs,
not prompt text, is the enforceable trust boundary.
Quality defects:
- to-sarif.mjs read a nonexistent `description` field; the canonical field is
`detail` and it is now propagated into `results[].message.text`. The rename
is applied across the validator, fixtures and tests. SARIF rule-level
shortDescription/fullDescription keys are schema-mandated and unchanged.
- security.yml now installs with `npm ci --ignore-scripts` so the audit path
never executes repository lifecycle scripts. ci.yml is deliberately
unchanged because it builds and tests the package.
- The `--skip-reachability` flag is removed. A test-only escape hatch
(`SECURITY_AUDIT_TEST_MODE=1`) replaces it, and a workflow invariant asserts
no workflow ever sets it.
- check-action-pins.mjs now walks the repository recursively and validates
composite action definitions (action.yml / action.yaml) in addition to
workflows.
- Instruction surfaces (AGENTS, CLAUDE, copilot-instructions and similar) are
explicitly denied from the model corpus.
- The Gitleaks release checksums were re-verified byte-for-byte against the
official goreleaser checksums asset for v8.30.1, and a provenance comment
recording the source URL sits above GITLEAKS_SHA256 in both workflows.
- Documents the squash/rebase merge assumption: the repository permits merge,
squash and rebase, and the latter two rewrite history so a pull request head
SHA is not reachable from main after merge. The reachability rule is
deliberately not relaxed; operators audit the resulting main commit.
Prompt vocabulary (categories, severities, confidences, caps) is now injected
into the templates from lib/constants.mjs at render time, so the prompt and the
validator cannot drift apart.
The AI layer remains deliberately ready-but-NOT_CONFIGURED. No credential is
stored or reused, no repository settings or secrets are mutated, and the
summary continues to report AI NOT_CONFIGURED rather than claiming a pass.
Validation: workflow YAML gate OK (3 files); action pins exit 0 (3 files);
security audit tests 54/54; offline dry run exit 0; lint 0 errors;
typecheck 0; build 0; vitest 56 files / 763 passed / 7 skipped.
AB#3219476
Co-authored-by: Copilot <copilot@github.com>
…nd attribute SARIF explicitly
Addresses the two pipeline-validation blockers filed as AB#3219526 against the
weekly security audit workflow (AB#3219476).
Blocker 1 - trusted controller vs audited target
Previously every job checked out the requested target commit at the workspace
root and then invoked scripts/security-audit/* from that checkout. For any
reachable-from-main ancestor predating this feature the helper scripts do not
exist, so the run either failed opaquely or would have executed whatever the
target commit happened to carry at those paths. Audit logic is now always
supplied by a controller checkout of the protected default branch at the
workspace root, and the audited commit is mounted read-only at target/:
- every job that runs a helper checks out the controller first (no ref
override, persist-credentials false) and then the target into target/;
the ordering is required because actions/checkout runs git clean -ffdx in
its destination directory
- collect-corpus.mjs gained --repo-root (default .) and reads the audited
tree from there while keeping manifest keys repository-relative
- check-action-pins.mjs is invoked with --dir target/.github/workflows
--root target
- gitleaks scans target; npm ci --ignore-scripts and npm audit run with
working-directory: target; setup-node cache: npm was dropped because the
lockfile is no longer at the workspace root
- codeql runs no helper, so it takes the target checkout only plus
init source-root: target
- dry-run.mjs forwards --repo-root end to end
Blocker 2 - result attribution
CodeQL and model SARIF uploads previously inherited the event SHA, which is the
default-branch tip rather than the audited commit. Both uploads now pass the
explicit checkout_path, ref and sha of the audited target. validate-target.mjs
publishes target_ref (refs/heads/main) and is_main_tip, surfaced through
validate-inputs outputs. Because the code scanning API documents sha as the
head of ref, a historical ancestor cannot be represented safely: the model
SARIF upload is gated on is_main_tip == 'true' and a dedicated step fails the
job closed for historical targets rather than misattributing findings or
publishing a raw findings artifact on a public repository.
Also fixed a latent defect surfaced while exercising a non-default scope: the
dry-run unresolved-placeholder assertion scanned the whole prompt buffer, which
legitimately contains ${{ }} expressions once workflow files enter the corpus.
The assertion is now scoped to trusted regions only (system prompt in full plus
the trusted suffix isolated at the closing corpus fence); the nonce-presence
assertion still spans the entire buffer.
Round 2 behaviour is preserved unchanged: schedule/empty-ref resolution to the
default-branch tip, per-run cryptographic corpus nonce fences, and the trusted
suffix that reasserts the output contract after the corpus. The AI layer
remains deliberately unconfigured and reports AI NOT_CONFIGURED.
Tests: 64 node:test cases (up from 54), including a regression that treats
ancestor 819431d - which contains none of the audit helpers - as a valid target,
assertions that no helper or npm invocation escapes target/, that controller
checkouts precede target checkouts, and that both SARIF paths carry explicit
ref/sha/checkout_path.
Validation: YAML parse x3 OK; action pins 0 violations across 3 files;
security:audit:test 64/64; security:audit:dry-run exit 0 (default and workflows
scopes); lint 0 errors / 12 pre-existing warnings; typecheck 0; build 0;
vitest 56 files / 763 passed / 7 skipped.
AB#3219476
AB#3219526
Co-authored-by: Copilot <copilot@github.com>
…orpus
The prompt-injection containment control documented in
docs/SECURITY-AUDIT.md ("agent instruction surfaces are never collected")
was never implemented in CORPUS_DENY_PATTERNS. The gap was masked
incidentally because ALLOWED_EXTENSIONS excludes `.md`, so the common
instruction files were already skipped by extension rather than by an
explicit control. That is fragile: widening the extension allowlist would
have silently re-admitted files that are written to be obeyed by a model.
Add extension-independent, path-anchored deny patterns for AGENTS.*,
CLAUDE.*, SKILL.*, copilot-instructions.*, .github/{instructions,agents,
prompts,chatmodes}/, .copilot/ and *.{instructions,agent,prompt,chatmode}.md,
and document the two distinct reasons entries appear in this list.
Add a regression test asserting each instruction surface is denied, that
denial holds for allowlisted extensions too, and that legitimate source
files remain eligible.
Validation: audit tests 65/65; dry-run exit 0 (server-core 40 files /
348275 B; workflows 17 files / neutralized 1); pins clean across 3 files;
YAML parse x3; lint 0 errors; typecheck, build and vitest (763 passed /
7 skipped) all green.
AB#3219476
Co-authored-by: Copilot <copilot@github.com>
Addresses the OSS review findings on the weekly security audit workflow.
Public egress (counts only)
- sanitize-findings.mjs: sanitizeGitleaks now emits only
{ kind, total, ruleCount, fileCount }. Rule identifiers and file paths
are no longer written to the uploaded summary or the job summary; the
redacted scan step log remains the restricted triage channel.
- security-audit.yml / security.yml: comments corrected to describe the
counts-only contract.
Canonical finding schema
- to-sarif.mjs: import SEVERITIES from constants.mjs, assert at module
load that every canonical severity is mapped in both SARIF_LEVEL and
SECURITY_SEVERITY, and throw on an unmapped severity instead of
silently defaulting to warning / 5.0. The prompt, validator, SARIF
converter, and tests now share one vocabulary sourced from
constants.mjs.
Egress ordering
- model-audit now declares needs on validate-inputs and secret-scan, so
no repository source is sent to the model provider until the secret
scan has succeeded.
Reproducible Copilot CLI install
- tools/copilot-cli/package.json pins @github/copilot exactly and is
private + UNLICENSED so it never enters the published tarball.
- The workflow replaces npm install -g with a fail-closed
npm ci --ignore-scripts from that manifest and passes the resolved
binary via copilot-cli-path. Absent lockfile fails the job.
- tools/copilot-cli/README.md documents licensing, the lock-generation
activation prerequisite, and verification steps.
- dependabot.yml watches /tools/copilot-cli.
Governance documentation
- docs/SECURITY-AUDIT.md: removed the unimplemented Foundry OIDC claim,
added an explicit CELA/Privacy approval gate, and added a COPILOT_PAT
governance table covering owner, scope, rotation, offboarding,
billing, provider terms, retention, and debug-log restrictions.
The model-assisted layer remains disabled by default and continues to
report AI NOT_CONFIGURED; the deterministic jobs are unchanged in
behaviour. No GitHub settings, secrets, or variables were modified.
AB#3219476
AB#3219526
Co-authored-by: Copilot <copilot@github.com>
…nalysis Documentation-only corrections from legal review of the weekly security audit workflow. No behavioural change: the model layer remains disabled by default, the toolchain lockfile guard remains fail-closed, and no repository or organisation settings are modified. Credential model (docs/SECURITY-AUDIT.md): - Remove the requirement for "a team alias, not a personal account". GitHub has no team-alias credential; a PAT is always bound to an account. The activation steps now require provisioning a team-owned managed service (machine) GitHub account first. - Require a Copilot Business or Copilot Enterprise seat on that account, and record that Individual/Pro seats are disallowed pending legal review. - Replace the credential obligations table with account, seat, named owners (at least two), least scope (Copilot Requests only), explicit expiry, rotation, an offboarding checklist, and a premium-request cost centre. - Remove the two rows that read as settled legal determinations. Activation determinations: - Add a table of seven open questions covering prompt/completion retention, data residency, provider terms and acceptable-use policy, model training, telemetry and provider-side logging, contributor disclosure sufficiency, and export/third-party review. Every row is marked "Not determined" and must be completed by legal and privacy review before activation. Nothing in this change asserts that any of them is answered or approved. Contributor disclosure (CONTRIBUTING.md): - Add an "Optional model-assisted security analysis" section, marked disabled by default, describing the bounded corpus of already-public git-tracked source relayed to GitHub Copilot and a third-party model provider, and stating that no repository metadata or untracked files are sent, no tools or writes are available to the model, output is advisory and redacted, and contributions never trigger the stage. - Point contributors at the full design document and invite them to raise concerns with a maintainer. Tests (scripts/security-audit/tests/workflow-invariants.test.mjs): - Four new invariants covering the managed service account requirement, the determinations table being recorded as open rather than approved, the contributor disclosure wording, and the model layer still being disabled everywhere in the repository. Audit suite is now 74 tests. AB#3219476 Co-authored-by: Copilot <copilot@github.com>
Privacy review follow-up for the weekly security audit workflow. Fail closed under debug logging (new) The model-audit job now aborts before any corpus is collected when ACTIONS_STEP_DEBUG, ACTIONS_RUNNER_DEBUG or runner.debug is truthy. Debug logging echoes step inputs and outputs into the Actions log, which is world-readable on a public repository, so it would flush the prompt corpus and the model response into public view. The guard runs after the credential check and before corpus collection, prompt assembly, CLI install and inference. Fix a P1 that was still open, not closed The review asked us to verify that gitleaks stdout was already suppressed. It was not. Both `gitleaks git` invocations wrote directly to the step log, and gitleaks emits one zerolog line per finding carrying File, Line, Commit, Author, Email, Date, Fingerprint and RuleID. `--redact` masks only the secret value, not that metadata, so every finding location and committer identity would have been published to a world-readable log. Both invocations now redirect stdout and stderr to a scratch file that is deleted unread, while preserving the exit status so the failure gate still fires. Narrow the model allowlist to one provider chain ALLOWED_MODELS and the workflow_dispatch choices now hold exactly claude-opus-5. Each model family is a different provider and subprocessor chain; only one chain is in scope for the pending privacy determination. Widening the allowlist requires its own CELA/Privacy review. Correct the log-visibility claims in the docs Three places implied Actions logs are collaborator-only. They are world-readable on a public repository. Secret triage is documented as a local `gitleaks git .` re-run matched against the published counts. The model layer stays disabled. SECURITY_AUDIT_AI_ENABLED is unset, the tools/copilot-cli lockfile guard still fails closed, and no legal or privacy determination is recorded as approved. Validation: 76/76 audit tests, YAML parse OK on all three workflows, action-pin scan clean, both dry runs exit 0. AB#3219476 Co-authored-by: Copilot <copilot@github.com>
OSS re-review follow-up for the weekly security audit workflow. Two trust-boundary defects and one documentation correction. Pin the controller checkout to the validated main tip workflow_dispatch lets any actor with write access choose the branch that supplies both the workflow YAML and every helper script it runs. Previously the controller checkout took actions/checkout's default, which is the event-selected ref, so a dispatch from an attacker's branch would have run that branch's collect-corpus.mjs, validate-response.mjs and sanitize-findings.mjs against the target. validate-target.mjs now resolves the origin/main tip exactly once and publishes it as `controller_sha`. Every job that invokes a helper pins its controller checkout to that commit, so the code that decides what is collected, what is redacted and what is published always comes from protected main -- never from the dispatch ref and never from the (possibly historical) target commit. The two jobs that cannot consume the output -- validate-inputs, which produces it, and summary, which runs with `if: always()` and may see it empty -- use the protected branch name directly rather than an empty ref that would silently fall back to the event ref. As defence in depth, the validator also refuses to run when GITHUB_EVENT_NAME is set and GITHUB_REF is not refs/heads/main. That step lives in the dispatched copy of the workflow, so an actor with write access could delete it; the substantive control is the pin above. Both variables are default runner variables, so no `env:` wiring is required, and the guard no-ops off Actions so local runs and tests are unaffected. Fail closed on symlinks in both walks Corpus collection enumerated tracked paths with `git ls-files -z` and read them with statSync, which follows links. A tracked symlink is a blob whose content is its target, so committing one that points at ../../secrets.env, or at a path outside the checkout, would have put out-of-tree content into the bounded corpus that is sent to the model. Collection now reads file modes via `git ls-files -s -z` and rejects mode 120000 outright, uses lstatSync so a filesystem/index disagreement is caught too, and realpath-checks every accepted path against the canonical checkout root to catch a symlinked *parent* directory that the per-entry mode check cannot see. Rejections abort the run rather than skipping the file: a corpus that quietly drops entries is harder to reason about than one that refuses to build. The recursive action-pin scanner had the mirror-image problem. It deliberately resolved symlinked directories so a linked workflow directory would still be scanned, which let a link into another checkout, into /etc, or back into itself pull foreign content into the scan -- or hide an unpinned composite action behind a shadowing link. It now throws on any symlink, skips non-regular files, applies the same realpath containment check, and tracks visited real directories so hard-linked or bind-mounted cycles terminate. Correct the Gitleaks triage documentation Two places implied rule identifiers and file paths appear in the public run output. They do not: the public summary carries counts only and the raw report is deleted inside the job. Triage is documented as a restricted local re-run at the target SHA the run audited, with an explicit instruction to keep locations off public surfaces until the credential has been rotated. The model layer stays disabled. SECURITY_AUDIT_AI_ENABLED is unset, the allowlist remains claude-opus-5 only, the debug-logging guard and the tools/copilot-cli lockfile guard still fail closed, and no legal or privacy determination is recorded as approved. Validation: 83/83 audit tests (49 pipeline, 34 workflow invariants), 763 repo tests, typecheck and build clean, eslint 0 errors, action-pin scan clean across 3 files, offline dry run exits 0 with AI status DRY_RUN. AB#3219476 Co-authored-by: Copilot <copilot@github.com>
Align the workflow and public documentation with the pending review state, pinned controller design, and exact model corpus contents. Strengthen the contributor-disclosure regression accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Validation refreshed at
The previous SARIF screenshots are no longer representative and have been removed from this comment. Detailed findings are not posted to PRs, logs, artifacts, code scanning, or external trackers; the optional model path can only submit through GitHub Private Vulnerability Reporting. |
PR microsoft#95 review follow-up. Automated security findings and exploit detail must never reach a public surface, so the audit no longer publishes them anywhere and instead files a single private vulnerability report per audited commit. Remove every public egress for a finding: delete the SARIF converter and its upload, drop the `security-events: write` grant, remove all workflow artifact uploads, and reduce every job summary to one of two fixed literals that name no scanner, path, rule, count or advisory. The custom CodeQL job is removed outright rather than left to scan and discard, because code scanning alerts are publicly visible on a public repository. Deterministic npm and Gitleaks output is written to a file, reduced to counts, then deleted; neither ever reaches the console. Add `submit-report.mjs`, which posts one aggregate report per target SHA through GitHub Private Vulnerability Reporting, deduplicated by exact summary across paginated triage and draft advisories. It prints only `report: submitted|existing|none|failed`, retries 5xx twice, and fails closed on every other error with no fallback channel. Submission runs in the same protected job as inference, after the tool-less model process exits and its response validates, because findings cannot cross jobs without an artifact or job output. The advisory credential is exposed to exactly two steps and never to inference. Both SECURITY_AUDIT_AI_ENABLED and SECURITY_AUDIT_PRIVATE_REPORTING_ENABLED must be set before the model job can run, so the protected environment is never created implicitly. A missing credential or disabled reporting fails before submission rather than passing green. Activation still requires repository owners to enable private reporting, provision the protected environment and its credential, and set both variables. None of that is done here; the model layer stays disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a scheduled/manual repository security-audit system, including deterministic scanners and an optional gated model review, without changing product runtime behavior.
Changes:
- Adds controller/target-isolated workflow with dependency, secret, and action-pin checks.
- Adds validated, redacted model findings with private vulnerability-report submission.
- Adds documentation, configuration, fixtures, tests, and pinned CLI tooling.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/copilot-cli/README.md | Updated as part of this pull request. |
| tools/copilot-cli/package.json | Updated as part of this pull request. |
| SECURITY.md | Updated as part of this pull request. |
| scripts/security-audit/validate-target.mjs | Updated as part of this pull request. |
| scripts/security-audit/validate-response.mjs | Updated as part of this pull request. |
| scripts/security-audit/tests/workflow-invariants.test.mjs | Updated as part of this pull request. |
| scripts/security-audit/tests/submit-report.test.mjs | Updated as part of this pull request. |
| scripts/security-audit/summarize.mjs | Updated as part of this pull request. |
| scripts/security-audit/submit-report.mjs | Updated as part of this pull request. |
| scripts/security-audit/sanitize-findings.mjs | Updated as part of this pull request. |
| scripts/security-audit/README.md | Updated as part of this pull request. |
| scripts/security-audit/prompt.md | Updated as part of this pull request. |
| scripts/security-audit/prompt-suffix.md | Updated as part of this pull request. |
| scripts/security-audit/lib/redaction.mjs | Updated as part of this pull request. |
| scripts/security-audit/lib/mini-yaml.mjs | Updated as part of this pull request. |
| scripts/security-audit/lib/controls.mjs | Updated as part of this pull request. |
| scripts/security-audit/lib/constants.mjs | Updated as part of this pull request. |
| scripts/security-audit/fixtures/unsafe-response.txt | Updated as part of this pull request. |
| scripts/security-audit/fixtures/synthetic-response.txt | Updated as part of this pull request. |
| scripts/security-audit/fixtures/malicious-delimiter.ts | Updated as part of this pull request. |
| scripts/security-audit/fixtures/malformed-response.txt | Updated as part of this pull request. |
| scripts/security-audit/fixtures/injection-sample.ts | Updated as part of this pull request. |
| scripts/security-audit/fixtures/fixture-manifest.json | Updated as part of this pull request. |
| scripts/security-audit/fixtures/dry-run-findings.json | Updated as part of this pull request. |
| scripts/security-audit/dry-run.mjs | Updated as part of this pull request. |
| scripts/security-audit/collect-corpus.mjs | Updated as part of this pull request. |
| scripts/security-audit/check-action-pins.mjs | Updated as part of this pull request. |
| scripts/security-audit/build-prompt.mjs | Updated as part of this pull request. |
| README.md | Updated as part of this pull request. |
| package.json | Updated as part of this pull request. |
| docs/SECURITY-AUDIT.md | Updated as part of this pull request. |
| CONTRIBUTING.md | Updated as part of this pull request. |
| .gitignore | Updated as part of this pull request. |
| .github/workflows/security.yml | Updated as part of this pull request. |
| .github/workflows/security-audit.yml | Updated as part of this pull request. |
| .github/workflows/ci.yml | Updated as part of this pull request. |
| .github/dependabot.yml | Updated as part of this pull request. |
| .github/CODEOWNERS | Updated as part of this pull request. |
Suppressed comments (25)
.github/workflows/security-audit.yml:217
- This public failure literal is used when
npm auditfails, but deterministic findings are never submitted to Private Vulnerability Reporting; the raw report is deleted and maintainers reproduce locally. Claiming that details were reported privately is therefore false and sends triage to an empty report queue. Use a truthful generic failure message consistently across the deterministic gates and documentation.
echo "Security audit: FAIL — details were reported privately to maintainers." >&2
exit 1
.github/workflows/security-audit.yml:470
- This debug refusal runs after
Require credentialshas already materializedSECURITY_ADVISORY_TOKENin a step environment. Under the threat model stated immediately above, debug logging can expose step environments before this guard refuses the run, so the guard cannot prevent the credential exposure. Move it before every credential-bound step, in addition to keeping the advisory token out of preflight.
- name: Refuse to run under debug logging
env:
STEP_DEBUG: ${{ secrets.ACTIONS_STEP_DEBUG }}
RUNNER_DEBUG_SECRET: ${{ secrets.ACTIONS_RUNNER_DEBUG }}
RUNNER_DEBUG_CONTEXT: ${{ runner.debug }}
.github/workflows/security-audit.yml:367
- The pin checker writes successful scan counts and, on violations, the target path, line, action reference, and violation count to stdout/stderr. Because this is a normal
runstep, those values are public Actions-log content, contrary to the no-path/no-count disclosure policy. Sink both streams here while preserving the exit status so only the generic summary is exposed.
node scripts/security-audit/check-action-pins.mjs \
--dir target/.github/workflows \
--root target
.github/workflows/security-audit.yml:496
collect-corpus.mjswrites the scope and file/byte/skipped/neutralized counts to stdout, and this step does not redirect them, so they appear in the public Actions log. The disclosure policy explicitly forbids counts and paths outside the runner. Suppress the helper output at this workflow boundary and retain only its exit status.
node scripts/security-audit/collect-corpus.mjs \
--scope "${AUDIT_SCOPE}" \
--repo-root target \
--out .security-audit/model
.github/workflows/security-audit.yml:514
build-prompt.mjsprints its output paths plus corpus and prompt byte counts. With no redirection, that metadata is written to the public Actions log, despite the documented no-path/no-count policy. Suppress this command's output in the workflow while allowing its non-zero exit to fail the step.
node scripts/security-audit/build-prompt.mjs \
--corpus .security-audit/model \
--out .security-audit/model
.github/workflows/security-audit.yml:563
validate-response.mjsprints accepted, rejected, and redaction counts to stdout, and this step passes stdout straight into the public Actions log. In particular, the accepted count reveals whether the model produced reportable findings, which conflicts with the no-count/generic-output contract. Suppress the helper output here and preserve only the exit status.
node scripts/security-audit/validate-response.mjs \
--response "${RESPONSE_FILE}" \
--manifest .security-audit/model/corpus-manifest.json \
--out .security-audit/model/report.json
.github/workflows/security-audit.yml:137
- The validator writes
target_sha,controller_sha, scope, model, and other normalized values to stdout; thisrunstep does not redirect them, so they become public Actions-log output even though the values are already exported throughGITHUB_OUTPUT. The documented public contract allows only the generic summary literals. Suppress both streams here while preserving the output-file writes and exit status.
node scripts/security-audit/validate-target.mjs \
--ref "${INPUT_REF:-}" \
--scope "${INPUT_SCOPE:-server-core}" \
--model "${INPUT_MODEL:-claude-opus-5}" \
--dry-run "${INPUT_DRY_RUN:-false}"
.github/workflows/security-audit.yml:184
npm ciperforms npm's automatic audit by default, and npm can print the resulting vulnerability count to the public log before the explicit JSON audit is redirected.--ignore-scriptsdoes not disable that audit. Disable install-time auditing here (--audit=false) so only the separately capturednpm audit --jsonpath can observe the result.
- name: Install dependencies without lifecycle scripts
working-directory: target
run: npm ci --ignore-scripts
.github/workflows/security-audit.yml:535
- This
npm cialso runs npm's automatic dependency audit unless disabled, which can print a vulnerability count (and related install diagnostics) to the public log. The model job's explicit disclosure policy should not rely on npm's default output. Add--audit=false; dependency auditing is not the purpose of this install.
- name: Install Copilot CLI
run: |
set -euo pipefail
if [ ! -f tools/copilot-cli/package-lock.json ]; then
echo "::error::tools/copilot-cli/package-lock.json is missing. Generate it on a host with direct registry.npmjs.org access (see tools/copilot-cli/README.md) before enabling the model-assisted audit."
exit 1
fi
( cd tools/copilot-cli && npm ci --ignore-scripts )
.github/workflows/security-audit.yml:197
- Unlike the corresponding audit in
security.yml:44, this invocation redirects only stdout. npm can emit warnings/errors to stderr, so a failed or unusual audit can still put registry/diagnostic output into the public log while the comments claim the raw audit output never reaches the console. Redirect stderr to an in-runner sink (or/dev/null) as well.
npm audit --audit-level=high --json > "${GITHUB_WORKSPACE}/.security-audit/npm-audit.json"
.github/workflows/security-audit.yml:613
- Because this is a SHA-only checkout with the default shallow fetch, it does not create
refs/remotes/origin/main. The followingnpm run security:audit:testinvokesvalidate-target.mjswithout test mode, so its default-ref tests fail atrequireMainTip()in this job. Fetch the controller withfetch-depth: 0(or run the tests in a checkout that includesorigin/main).
persist-credentials: false
.github/workflows/security-audit.yml:366
- The documented historical-target support includes commits from before
.github/workflowsexisted, butcheck-action-pins.mjscallscollectFileson this path and fails when the directory is absent. Such a target cannot reach the composite-action scan or produce a valid audit result. Treat a missing workflow directory as an empty directory before invoking the checker.
--dir target/.github/workflows \
.github/workflows/security.yml:159
- The per-PR Gitleaks job never sends its findings to PVR, so this message falsely tells users that secret-scan details were privately reported. Use a truthful generic failure message and update the duplicated policy text consistently.
echo "Security audit: FAIL — details were reported privately to maintainers." >&2
exit 1
.github/workflows/security.yml:30
- The install step also leaves npm's automatic audit enabled. npm may print vulnerability counts to the public Actions log, bypassing the sanitizer used by the explicit audit step below;
--ignore-scriptsdoes not change this. Add--audit=falseto the install and keep the JSON audit as the only audit invocation.
# `--ignore-scripts` keeps dependency lifecycle scripts (install/postinstall)
# from executing on a runner whose only job is to read the lockfile. `npm
# audit` resolves advisories from package-lock.json and does not need a
# built dependency tree, so nothing here depends on those scripts running.
- run: npm ci --ignore-scripts
.github/workflows/security.yml:55
- This workflow runs on
pull_request, whose checkout is the PR merge commit, soscripts/security-audit/sanitize-findings.mjsis supplied by the untrusted PR. The raw npm audit report still exists when this command runs; a PR can modify the sanitizer to print or exfiltrate it to the public log before deletion. Execute the sanitizer from a trusted controller checkout (or inline the reduction) rather than from the PR tree.
node scripts/security-audit/sanitize-findings.mjs \
--kind npm-audit \
--in .security-audit/npm-audit.json \
--out .security-audit/npm-audit-summary.json
.github/workflows/security.yml:148
- This is the second instance of executing
sanitize-findings.mjsfrom the PR checkout. Here the raw Gitleaks report contains matched secret material, so a modified PR sanitizer could replay it into the public Actions log. Use the trusted controller copy for this invocation as well; the current redirect/deletion logic cannot protect against a modified sanitizer.
node scripts/security-audit/sanitize-findings.mjs \
--kind gitleaks \
--in .security-audit/gitleaks.json \
--out .security-audit/gitleaks-summary.json
CONTRIBUTING.md:70
- This contributor guidance repeats the private-report claim for the generic failure result, although deterministic failures are not sent to PVR. Update it to explain that the result is generic and that only validated model findings use private reporting.
`Security audit: FAIL — details were reported privately to maintainers.`
SECURITY.md:45
- The public security policy repeats a claim that every failed audit has privately reported details, but dependency, Gitleaks, and action-pin failures have no PVR submission. This can misdirect incident triage; describe the public result truthfully and reserve private-report wording for validated model findings.
Public workflow output for a security audit run is limited to one of two literals:
`Security audit: PASS` or
`Security audit: FAIL — details were reported privately to maintainers.`
docs/SECURITY-AUDIT.md:33
- This documented failure literal is also used for deterministic scanner failures, whose details are not submitted through PVR. It is therefore misleading to tell maintainers that details were reported privately; the documentation should distinguish deterministic failures from the optional model-report path and use a truthful generic literal.
> - `Security audit: FAIL — details were reported privately to maintainers.`
package.json:53
- This adds the audit test command, but
.github/workflows/ci.ymlrunsnpm testand never invokesnpm run security:audit:test; the new validators, redaction rules, and report submitter therefore have no automated PR gate. Add this suite to the CI workflow (or make the CI entry point invoke it) so these security controls cannot regress unnoticed.
"security:audit:test": "node --test \"scripts/security-audit/tests/*.test.mjs\"",
scripts/security-audit/README.md:47
- The script README says the generic failure result has privately reported details even for deterministic scanner failures, which have no PVR submission path. This operational instruction is inaccurate and should be aligned with the truthful generic failure message.
Security audit: FAIL — details were reported privately to maintainers.
scripts/security-audit/lib/redaction.mjs:31
- The Windows rule only recognizes drive-letter paths and accepts UNC paths such as
\\server\share\secret.txt. That is also an absolute filesystem path and can be persisted in the private report despite the validator contract. Include the UNC form in the rejection pattern.
{ label: 'absolute-path-windows', pattern: /\b[A-Za-z]:\\(?:[^\s"'`]+)/ },
scripts/security-audit/submit-report.mjs:387
submittedversusnoneis a public side channel for whether the model produced any findings, andexistingreveals the presence of a prior private report. The documented public contract permits only the generic pass/fail result. Suppress this status line in the Actions invocation while preserving any local CLI contract.
scripts/security-audit/validate-target.mjs:270- These lines print
target_sha,controller_sha,scope, and the model to the Actions log on every successful run. That contradicts the documented public-output contract, which withholds commit/scope metadata and exposes only a generic verdict. Write these values only to$GITHUB_OUTPUT, retaining stdout output for local invocations.
tools/copilot-cli/README.md:72 - Because the lockfile is intentionally absent, configuring the four activation prerequisites described for this PR still starts the model job and guarantees that
Install Copilot CLIexits before inference. Make the verified lockfile a documented activation prerequisite or commit it before claiming the optional review can be activated.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+13
to
+16
| workflow_dispatch: | ||
| inputs: | ||
| ref: | ||
| description: 'Full 40-character commit SHA to audit. Must be reachable from main. Defaults to the main branch tip.' |
Comment on lines
+585
to
+588
| node scripts/security-audit/submit-report.mjs \ | ||
| --report .security-audit/model/report.json \ | ||
| --sha "${TARGET_SHA}" \ | ||
| --repo "${GITHUB_REPOSITORY}" |
Comment on lines
+442
to
+445
| - name: Require credentials | ||
| env: | ||
| COPILOT_PAT: ${{ secrets.COPILOT_PAT }} | ||
| SECURITY_ADVISORY_TOKEN: ${{ secrets.SECURITY_ADVISORY_TOKEN }} |
Comment on lines
+326
to
+327
| echo "Security audit: FAIL — details were reported privately to maintainers." >&2 | ||
| exit 1 |
Comment on lines
+182
to
+184
| - name: Install dependencies without lifecycle scripts | ||
| working-directory: target | ||
| run: npm ci --ignore-scripts |
Comment on lines
+288
to
+290
| if (response.status >= 500 && attempt < REPORT_RETRY_LIMIT) { | ||
| await sleepImpl(REPORT_RETRY_DELAY_MS); | ||
| continue; |
Comment on lines
+336
to
+337
| if (result.data.length < PAGE_SIZE) break; | ||
| } |
Comment on lines
+247
to
+249
| process.stdout.write( | ||
| `security-audit: accepted=${result.accepted.length} rejected=${result.rejected.length} redactions=${result.redactions.length}\n`, | ||
| ); |
| } | ||
|
|
||
| const file = typeof finding.file === 'string' ? finding.file.trim() : ''; | ||
| const entry = manifest.files?.[file]; |
Comment on lines
+139
to
+141
| const line = Number(finding.line); | ||
| if (!Number.isInteger(line) || line < 1) reasons.push('line-not-a-positive-integer'); | ||
| else if (entry && line > entry.lines) reasons.push('line-out-of-range'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
npm audit, checksum-pinned Gitleaks, and action-pin enforcementmaincontroller separate from the audited target and retain the existing corpus, symlink, prompt-injection, and response-validation controlsTracks AB#3219476.
Validation
npm run security:audit:test- 124 passednpm run security:audit:dry-run- passed without a model, credential, network request, or published outputnpm run security:audit:pins- all actions SHA-pinnednpm run ci- 763 passed, 7 skippedgit diff --checkActivation boundary
The model-assisted job remains disabled by default. Enabling it requires:
security-audit-private-reportenvironment with required reviewers and amain-only deployment rule.SECURITY_ADVISORY_TOKENwith only Repository security advisories write access (or a short-lived GitHub App token).SECURITY_AUDIT_AI_ENABLED=trueandSECURITY_AUDIT_PRIVATE_REPORTING_ENABLED=true.Until all four are configured, the model job does not run. If private submission fails after activation, the job fails without publishing the finding elsewhere.
There is no product runtime or published-package change.