batch 2 of 6: CI safe-patterns + supply-chain absorption#52
batch 2 of 6: CI safe-patterns + supply-chain absorption#52AceHack wants to merge 5 commits intoLucent-Financial-Group:mainfrom
Conversation
Scope: security-adjacent CI surfaces from the speculative branch
(resume-diff workflow, scorecard workflow, two safe-patterns
guides, Semgrep rule 17, Dependabot config, incident playbook).
Per `docs/research/speculative-branch-landing-plan-2026-04-22.md`
batch 2.
Files landed:
- `.github/workflows/resume-diff.yml` (166 LoC) — PR comment
producing a claim-level diff on FACTORY-RESUME.md +
SHIPPED-VERIFICATION-CAPABILITIES.md. Not a gate. Complies
with the new safe-patterns checklist: env-block consumption,
SHA-pinned action, minimized permissions, no inline ${{}} in
run scripts, inputs restricted to SHAs + PR number.
`actionlint` clean locally.
- `.github/workflows/scorecard.yml` — OpenSSF Scorecard weekly
run with SARIF upload to Security tab. SHA-pinned, no
inline `${{}}` in run: scripts. `actionlint` clean.
- `docs/security/GITHUB-ACTIONS-SAFE-PATTERNS.md` — pre-write
checklist + threat model + trusted/untrusted context table +
do/don't pairs. Derived from github.blog vulnerability-
research guidance on workflow-injection. Cross-refs the
factory's existing triple-layer lint (actionlint + CodeQL-
actions + Semgrep).
- `docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md` — SLSA-level
guidance + canonical incidents (CVE-2025-30066 tj-actions;
Trivy TeamPCP attack 2026-03-19) + SHA-pin-everything policy
+ content-review-is-load-bearing rule (a SHA-256 of a
compromised release binary is still a valid SHA-256).
Fixed MD007 ul-indent violation on commit (4->2 spaces for
first-level sub-bullets) — the only markdownlint finding.
- `.semgrep.yml` — rule 17 `gha-untrusted-in-run-line` — regex
match on `run:` lines inlining attacker-controlled
`github.event.*` / `github.head_ref` values. Closes the
overclaim in the safe-patterns doc (which listed Semgrep as
a third enforcement layer). Fires per-PR ahead of CodeQL's
weekly cadence.
- `docs/security/INCIDENT-PLAYBOOK.md` — expanded A-block
references for Trivy TeamPCP + tj-actions incidents; the
"why content review matters" thread.
- `.github/dependabot.yml` — +11 lines (extra ecosystems /
interval tightening — carries the new scorecard-action
dep path).
Verification:
- `actionlint` clean on both new workflow files.
- `markdownlint-cli2` clean on all three docs/security/*.md
files after MD007 fix.
- `shellcheck` — no new `.sh` files in this batch.
Causally linked to:
- FACTORY-HYGIENE row 24 (CI safe-patterns audit, high-risk).
- Aaron's 2026-04-22 directive to absorb GitHub Security Lab
workflow-injection guidance so new workflows are secure by
default.
- Batch 1 (PR Lucent-Financial-Group#49) established the drain pattern;
PR Lucent-Financial-Group#50 proved fork-PR flow; this batch continues the
speculative-branch drain.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR lands “batch 2 of 6” from the speculative-branch plan, focused on hardening CI and documenting security-safe patterns for GitHub Actions and dependency/supply-chain hygiene.
Changes:
- Add two new security guidance docs covering GitHub Actions workflow-injection prevention and supply-chain safe patterns.
- Add two new GitHub Actions workflows: OpenSSF Scorecard (SARIF upload) and a PR-helper that posts a resume-claim diff comment.
- Extend Semgrep with a new rule to catch inline untrusted
${{ github.* }}expansions on single-linerun:steps, plus small CI hygiene/doc updates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md | New in-repo checklist and threat model for dependency/supply-chain hygiene. |
| docs/security/INCIDENT-PLAYBOOK.md | Expand Playbook A canonical-incident references (mutable-tag class). |
| docs/security/GITHUB-ACTIONS-SAFE-PATTERNS.md | New workflow-injection safe-patterns checklist and enforcement overview. |
| .semgrep.yml | Add rule to flag untrusted GitHub context interpolation in run: lines. |
| .github/workflows/scorecard.yml | New weekly OpenSSF Scorecard workflow with SARIF upload. |
| .github/workflows/resume-diff.yml | New PR helper workflow that comments a claim-level diff for resume-related docs. |
| .github/dependabot.yml | Add clarifying comments about uv-managed Python tooling vs Dependabot support. |
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
There was a problem hiding this comment.
On fork PRs, workflows triggered by pull_request commonly receive a read-only GITHUB_TOKEN, so gh pr comment can fail even if pull-requests: write is declared. To avoid noisy red runs, consider gating the comment step/job to same-repo PRs (or handling the failure explicitly), or document the required repo setting if write tokens for fork PRs are enabled intentionally.
|
|
||
| CLAIM_LINES="$(printf '%s\n' "$RAW_DIFF" \ | ||
| | grep -E '^[+-][^+-]' \ | ||
| | grep -E '^[+-]\s*(- \*\*|\| |#{2,4} |.*\b(ships?|shipped|verified|proven|complete[ds]?|honest|already absorbed|implement(ed|s)?|in[- ]repo evidence)\b)' \ |
There was a problem hiding this comment.
grep -E doesn’t support \s (whitespace) or \b (word boundary), so this claim-line filter won’t match what it intends on ubuntu-22.04 (GNU grep treats these as literal characters). Use POSIX character classes like [[:space:]] and a word-boundary alternative (or switch to grep -P / perl) so the detection is actually effective under set -euo pipefail.
| | grep -E '^[+-]\s*(- \*\*|\| |#{2,4} |.*\b(ships?|shipped|verified|proven|complete[ds]?|honest|already absorbed|implement(ed|s)?|in[- ]repo evidence)\b)' \ | |
| | grep -E '^[+-][[:space:]]*(- \*\*|\| |#{2,4} |.*(^|[^[:alnum:]_])(ships?|shipped|verified|proven|complete[ds]?|honest|already absorbed|implement(ed|s)?|in[- ]repo evidence)([^[:alnum:]_]|$))' \ |
| rebuild-trust signals. | ||
|
|
||
| Re-read against current revisions of these sources every 5-10 | ||
| rounds (FACTORY-HYGIENE row 41, same cadence as GHA safe-patterns). |
There was a problem hiding this comment.
This cadence pointer says “FACTORY-HYGIENE row 41” but docs/FACTORY-HYGIENE.md row 41 is “Orthogonal-axes audit”, not a supply-chain or dependency-ingress re-read. Please either add the intended hygiene row or update this reference to the correct row so the doc doesn’t send readers to the wrong audit.
| rounds (FACTORY-HYGIENE row 41, same cadence as GHA safe-patterns). | |
| rounds (same cadence as GHA safe-patterns; FACTORY-HYGIENE | |
| dependency-ingress re-read cadence). |
| decision itself is **content review at first pin**. A SHA-256 that | ||
| points at malicious code is still malicious; a hand-verified | ||
| script run `curl | bash`-style after a careful read is safe. | ||
|
|
||
| In this factory, Aaron's standing policy (2026-04-22) is: *"never |
There was a problem hiding this comment.
This section (a) endorses curl | bash as “safe” if hand-verified, which conflicts with the doc’s own later protocol to download-to-disk first, and (b) includes a direct individual name attribution. Reword to consistently recommend “download → review → execute” (no pipe-to-shell phrasing) and replace personal-name attribution with a role reference (e.g., “human maintainer policy” / “factory policy”).
|
|
||
| # Python CLI tools (semgrep, ruff) live in | ||
| # `tools/setup/manifests/uv-tools`, installed via | ||
| # `common/python-tools.sh`. Dependabot does not support the |
There was a problem hiding this comment.
The referenced script path looks incorrect: the repo path is tools/setup/common/python-tools.sh, but this comment refers to common/python-tools.sh without the tools/setup/ prefix. Please fix the path so readers can find the script and pointer-integrity audits don’t flag it.
| # `common/python-tools.sh`. Dependabot does not support the | |
| # `tools/setup/common/python-tools.sh`. Dependabot does not support the |
| set from a trusted literal), `secrets.*` | ||
|
|
||
| "Trusted" here means GitHub sets the value, not a user. |
There was a problem hiding this comment.
The “Trusted context” list includes secrets.*, but later the checklist correctly states secrets should never be interpolated into run: strings. This is contradictory guidance in the same doc; either remove secrets.* from the trusted-to-inline list or add an explicit note that secrets are injection-safe but still must not be inlined due to log exposure.
| set from a trusted literal), `secrets.*` | |
| "Trusted" here means GitHub sets the value, not a user. | |
| set from a trusted literal) | |
| "Trusted" here means GitHub sets the value, not a user. `secrets.*` | |
| are not attacker-controlled for workflow-injection purposes, but | |
| they must still never be interpolated directly into a `run:` string; | |
| pass them via `env:` or an action input instead. |
| - **Cadenced audit target** — FACTORY-HYGIENE row 40 cadences a | ||
| re-read against the blog's current revision so drift is caught. |
There was a problem hiding this comment.
This references “FACTORY-HYGIENE row 40” as the cadence for re-auditing the GitHub Security Lab guidance, but docs/FACTORY-HYGIENE.md row 40 is “GitHub-settings drift detector”. Either add the intended hygiene row (if this is meant to be tracked there) or update the reference to the correct existing row so the cadence pointer is accurate.
| Incident Playbook A, and the scanner-landscape research | ||
| doc (`docs/research/vuln-and-dep-scanner-landscape-2026- | ||
| 04-22.md`) — which defers Trivy adoption pending | ||
| rebuild-trust signals. |
There was a problem hiding this comment.
The referenced file path is split across lines (...-2026- then 04-22.md), which breaks copy/paste and makes the cross-reference ambiguous. Keep paths unbroken (no hyphenation) so pointer-integrity audits and readers can reliably follow the link.
| Incident Playbook A, and the scanner-landscape research | |
| doc (`docs/research/vuln-and-dep-scanner-landscape-2026- | |
| 04-22.md`) — which defers Trivy adoption pending | |
| rebuild-trust signals. | |
| Incident Playbook A, and the scanner-landscape research doc | |
| (`docs/research/vuln-and-dep-scanner-landscape-2026-04-22.md`) — | |
| which defers Trivy adoption pending rebuild-trust signals. |
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | ||
| PATTERNS.md` §"Content review is the load-bearing step" |
There was a problem hiding this comment.
The file path in backticks is broken across lines (SUPPLY-CHAIN-SAFE- + PATTERNS.md), which makes the reference incorrect if copied. Avoid hard-wrapping/hyphenating file paths; keep the full path on one line.
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | |
| PATTERNS.md` §"Content review is the load-bearing step" | |
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md` §"Content review is the load-bearing step" |
| # not advisories. See `docs/research/vuln-and-dep-scanner- | ||
| # landscape-2026-04-22.md` adopt-now item #3 for the rationale. |
There was a problem hiding this comment.
This comment hard-wraps the referenced doc path across lines (docs/research/vuln-and-dep-scanner- + landscape-...), which breaks copy/paste and makes pointer-integrity checking harder. Keep repository paths unbroken on a single line.
| # not advisories. See `docs/research/vuln-and-dep-scanner- | |
| # landscape-2026-04-22.md` adopt-now item #3 for the rationale. | |
| # not advisories. See `docs/research/vuln-and-dep-scanner-landscape-2026-04-22.md` | |
| # adopt-now item #3 for the rationale. |
| # judgment stays with the reviewer and with Aaron (honesty- | ||
| # floor owner). |
There was a problem hiding this comment.
This comment uses a direct contributor name (“Aaron”), but repo guidance is to avoid name attribution in code/docs and use role references (e.g., “human maintainer”) except in explicitly exempt surfaces. Please rewrite this to use the role reference here.
| # judgment stays with the reviewer and with Aaron (honesty- | |
| # floor owner). | |
| # judgment stays with the reviewer and with the human | |
| # maintainer (honesty-floor owner). |
| points at malicious code is still malicious; a hand-verified | ||
| script run `curl | bash`-style after a careful read is safe. | ||
|
|
||
| In this factory, Aaron's standing policy (2026-04-22) is: *"never | ||
| run a script you download without checking it for vulnerability, | ||
| trojans and things of that nature even like gist and stuff, it's | ||
| fine to download and run bash and things like that just validate | ||
| them first."* So the actual author-time protocol is: | ||
|
|
||
| 1. **Download to disk**, do not execute. | ||
| 2. **Read the script in full** — check for sudo / privilege | ||
| escalation, data exfiltration (`curl -X POST`, `nc`, `scp` to | ||
| non-project hosts), shell-metacharacter injection, opaque | ||
| base64 blobs, cryptominers, calls to suspicious domains, | ||
| trojans masquerading as legitimate installers. | ||
| 3. **Execute if clean**; record the validation decision in the | ||
| commit message or manifest comment. | ||
| 4. **SHA-256-pin the validated content** — the pin is then a | ||
| cache of your review. Any bump invalidates the cache and | ||
| forces a re-read. | ||
|
|
||
| The delivery mechanism (`curl | bash` vs `curl -o path && bash | ||
| path`) is not the risk. The risk is unvalidated content. |
There was a problem hiding this comment.
This section both (1) frames curl | bash as “safe” if reviewed and (2) attributes policy to a named individual. In this repo, guidance is to avoid pipe-to-shell recommendations (prefer download-to-disk then execute) and to avoid name attribution in docs (use role refs like “human maintainer” outside exempt surfaces). Please reword to discourage pipe-to-shell even after review, and replace the named attribution with a role reference (or move the quote to an exempt/historical surface).
| points at malicious code is still malicious; a hand-verified | |
| script run `curl | bash`-style after a careful read is safe. | |
| In this factory, Aaron's standing policy (2026-04-22) is: *"never | |
| run a script you download without checking it for vulnerability, | |
| trojans and things of that nature even like gist and stuff, it's | |
| fine to download and run bash and things like that just validate | |
| them first."* So the actual author-time protocol is: | |
| 1. **Download to disk**, do not execute. | |
| 2. **Read the script in full** — check for sudo / privilege | |
| escalation, data exfiltration (`curl -X POST`, `nc`, `scp` to | |
| non-project hosts), shell-metacharacter injection, opaque | |
| base64 blobs, cryptominers, calls to suspicious domains, | |
| trojans masquerading as legitimate installers. | |
| 3. **Execute if clean**; record the validation decision in the | |
| commit message or manifest comment. | |
| 4. **SHA-256-pin the validated content** — the pin is then a | |
| cache of your review. Any bump invalidates the cache and | |
| forces a re-read. | |
| The delivery mechanism (`curl | bash` vs `curl -o path && bash | |
| path`) is not the risk. The risk is unvalidated content. | |
| points at malicious code is still malicious; even after review, | |
| do not execute a freshly downloaded script via `curl | bash`. | |
| Download it to disk first, inspect the exact bytes you plan to | |
| run, and only then execute the local file if it passes review. | |
| In this factory, the human maintainer's standing policy | |
| (2026-04-22) is: *"never run a script you download without | |
| checking it for vulnerability, trojans and things of that nature | |
| even like gist and stuff"* — applied here as a download-review- | |
| execute workflow, not a pipe-to-shell exception. So the actual | |
| author-time protocol is: | |
| 1. **Download to disk**, do not execute from the network stream. | |
| 2. **Read the script in full** — check for sudo / privilege | |
| escalation, data exfiltration (`curl -X POST`, `nc`, `scp` to | |
| non-project hosts), shell-metacharacter injection, opaque | |
| base64 blobs, cryptominers, calls to suspicious domains, | |
| trojans masquerading as legitimate installers. | |
| 3. **Execute the reviewed local file if clean**; record the | |
| validation decision in the commit message or manifest comment. | |
| 4. **SHA-256-pin the validated content** — the pin is then a | |
| cache of your review. Any bump invalidates the cache and | |
| forces a re-read. | |
| The key risk is unvalidated content, and pipe-to-shell removes a | |
| safety boundary during validation. Prefer download-to-disk, then | |
| review, then execute. |
| # | ||
| # Action-pin content-review (per `docs/security/SUPPLY-CHAIN- | ||
| # SAFE-PATTERNS.md`): ossf/scorecard-action v2.4.3 tagged | ||
| # 2025-09-30 by sschrock@google.com (OpenSSF maintainer), |
There was a problem hiding this comment.
This header comment includes an individual’s email address (ss...@google.com). Beyond name-attribution guidance, embedding personal identifiers in workflow comments tends to age badly and can be unnecessary PII in-repo. Consider replacing it with an organization/role reference (e.g., “OpenSSF maintainer”) and/or a link to the tagged release/verification evidence instead of a specific email.
| # 2025-09-30 by sschrock@google.com (OpenSSF maintainer), | |
| # 2025-09-30 by an OpenSSF maintainer, |
| # that form the factory's "job-interview honesty" surface per | ||
| # `memory/feedback_factory_resume_job_interview_honesty_only_direct_experience.md`. |
There was a problem hiding this comment.
The referenced memory file memory/feedback_factory_resume_job_interview_honesty_only_direct_experience.md does not appear to exist in the repo (no matching file under memory/). Either add the missing memory entry or update this reference to the correct existing path so the workflow header doesn’t point at a dead link.
| # that form the factory's "job-interview honesty" surface per | |
| # `memory/feedback_factory_resume_job_interview_honesty_only_direct_experience.md`. | |
| # that form the factory's "job-interview honesty" surface. |
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | ||
| PATTERNS.md` §"Content review is the load-bearing step" | ||
| matters: a SHA-256 of a compromised release is still a | ||
| valid SHA-256. We do not currently consume `trivy-action`; |
There was a problem hiding this comment.
The file path in backticks is split across lines (SUPPLY-CHAIN-SAFE- then PATTERNS.md), which breaks copy/paste and makes the reference ambiguous. Keep the full path on one line (or use a markdown link) so the citation stays mechanically usable during an incident.
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | |
| PATTERNS.md` §"Content review is the load-bearing step" | |
| matters: a SHA-256 of a compromised release is still a | |
| valid SHA-256. We do not currently consume `trivy-action`; | |
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md` | |
| §"Content review is the load-bearing step" matters: a | |
| SHA-256 of a compromised release is still a valid | |
| SHA-256. We do not currently consume `trivy-action`; |
| In this factory, Aaron's standing policy (2026-04-22) is: *"never | ||
| run a script you download without checking it for vulnerability, | ||
| trojans and things of that nature even like gist and stuff, it's | ||
| fine to download and run bash and things like that just validate | ||
| them first."* So the actual author-time protocol is: |
There was a problem hiding this comment.
Direct contributor name attribution was introduced here (and similarly in the new resume-diff workflow header). Repo convention is to avoid direct contributor names in docs/code/skills and use role references instead (e.g., “human maintainer”); names should live under memory/persona/** (docs/AGENT-BEST-PRACTICES.md:284-290). Please replace the name with a role reference and move any attribution/quote provenance to an allowed location.
| In this factory, Aaron's standing policy (2026-04-22) is: *"never | |
| run a script you download without checking it for vulnerability, | |
| trojans and things of that nature even like gist and stuff, it's | |
| fine to download and run bash and things like that just validate | |
| them first."* So the actual author-time protocol is: | |
| In this factory, the standing policy is: never run a script you | |
| download without checking it for vulnerabilities, trojans, or | |
| other hostile behaviour first. Downloading and running a script | |
| can be acceptable only after that validation. So the actual | |
| author-time protocol is: |
| - name: Post PR comment | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| DIFF_FILE: ${{ steps.diff.outputs.diff_file }} | ||
| run: | | ||
| set -euo pipefail | ||
| gh pr comment "$PR_NUMBER" --body-file "$DIFF_FILE" |
There was a problem hiding this comment.
gh pr comment will fail on fork PRs when the workflow is triggered via pull_request, because GITHUB_TOKEN is read-only for forks and cannot write PR comments. Since this step runs under set -euo pipefail, the whole workflow will go red for forked contributions. Make comment posting conditional on same-repo PRs, or switch to a hardened pull_request_target design (while ensuring you never execute PR-provided code) so the workflow behaves reliably.
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | ||
| PATTERNS.md` §"Content review is the load-bearing step" | ||
| matters: a SHA-256 of a compromised release is still a | ||
| valid SHA-256. We do not currently consume `trivy-action`; | ||
| scanner-adoption decisions are tracked in |
There was a problem hiding this comment.
Inline code spans can’t contain newlines; the backticked file path is split across lines (docs/security/SUPPLY-CHAIN-SAFE- / PATTERNS.md), which breaks Markdown rendering (and may confuse the reference). Keep the full path inside a single pair of backticks (or turn it into a normal link) without a hard line break.
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE- | |
| PATTERNS.md` §"Content review is the load-bearing step" | |
| matters: a SHA-256 of a compromised release is still a | |
| valid SHA-256. We do not currently consume `trivy-action`; | |
| scanner-adoption decisions are tracked in | |
| which underscores why `docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md` | |
| §"Content review is the load-bearing step" matters: a | |
| SHA-256 of a compromised release is still a valid SHA-256. | |
| We do not currently consume `trivy-action`; scanner-adoption | |
| decisions are tracked in |
* Round 44 tick-history: auto-loop-2 PR refresh row Second post-compaction tick. PR #91 refresh after PR #90 merged 4ac3ec3 on main mid-tick; PR #46 refresh after stale-local reset (bc93188..63720e5). Fork PRs #88/#85/#52/#54 noted as un-refreshable from current agent harness. Tick-commits-on-PR-branch = live-loop class (row 112) preserved via separate branch land-tick-history-autoloop-2-append off origin/main. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * tick-history row fixes — Copilot findings on PR #92 Three fixes to the auto-loop-2 row: 1. Drop "AceHack" handle → "fork ownership outside the canonical repo" (BP-L284-L290 compliance) 2. Full ISO8601 timestamp (2026-04-22T04:20:00Z) per file schema 3. Drop self-referential "(row 112 of this file)" Pre-check grep expanded to include "acehack" handle — prior grep only caught "aaron" + memory/* refs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Batch 2 of the 6-batch speculative-branch drain plan (`docs/research/speculative-branch-landing-plan-2026-04-22.md`). Security-adjacent CI surfaces.
New files (4):
Modified files (3):
Verification
Test plan
References