Skip to content

chore: supply chain update - 3#121

Merged
mohandast52 merged 8 commits into
mainfrom
mohan/supply-chain-update-3
May 11, 2026
Merged

chore: supply chain update - 3#121
mohandast52 merged 8 commits into
mainfrom
mohan/supply-chain-update-3

Conversation

@mohandast52
Copy link
Copy Markdown
Contributor

Summary

Phase 3 (final) of the supply-chain hardening rollout (see .plans/supply-chain-update.md, Tier 2 + parts of Tier 4). Stacked on #120, which stacks on #119. Will rebase to main once those merge.

Adds the regression-prevention layer: every future PR is automatically audited for new vulnerabilities, new install hooks, lockfile drift, and leaked secrets. All new gates land advisory-only — branch protection isn't being changed here, so failing gates won't block merging until you promote them in repo Settings.

Closes T5 (future-leak detection) from the threat model.

Changes

Workflows (added)

  • .github/workflows/supply-chain.yml — runs yarn audit:prod + install-hook gate at root, plus lockfile-lint matrixed over all 13 paths, with all-checks-passed aggregator job.
  • .github/workflows/gitleaks.yml — secret scan on every push + PR, with SHA-256 verified gitleaks 8.30.1 binary (551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb, cross-checked against townhall-kpis and upstream checksums.txt).

Scripts (added)

  • scripts/audit.mjs — wraps yarn audit --json with allowlist suppression. Critical: exposed as yarn audit:prod, NOT yarn audit (yarn 1.x's built-in shadows same-named scripts in package.json). Time-bounded suppression: each allowlist entry needs id, reason, added, review (all required); expired entries print a CI warning but don't fail.
  • scripts/audit-install-hooks.mjs — enumerates packages in node_modules declaring non-trivial pre/install/postinstall scripts, diffs against .supply-chain/install-hooks.allowlist. Drift in either direction (new hook OR removed hook) fails the job — the latter catches stale allowlist entries.

Allowlists (added, baselined)

  • .supply-chain/audit-allowlist.json23 unique high/critical advisories baselined as transitive of @graphprotocol/graph-cli (latest stable 0.98.1 doesn't refresh them). 90-day review cadence.
  • .supply-chain/install-hooks.allowlistempty. Notable finding: graph-cli's transitives at root are pure JS, so the install-hook attack surface at root is zero. Useful baseline for catching future regressions.

Configuration (changed)

  • .nvmrc — pins Node 22.18.0, resolves prior Node 20/24 drift between test.yaml (was 20) and deploy-subgraph.yaml (was 24).
  • package.json — adds engines.node: "22.x", packageManager: "yarn@1.22.22", and audit:prod / audit:install-hooks script aliases.
  • Both workflows: switched to node-version-file: .nvmrc and Corepack activation with version assertion (yarn --version must equal 1.22.22 or job fails). Also tightened deploy install to --frozen-lockfile.

Documentation (added/updated)

  • SUPPLY-CHAIN-SECURITY.md — threat model (T1-T5), secrets inventory, controls reference, response playbook, repo-specific watches.
  • CLAUDE.md — Supply chain & security section pointing at the docs + the audit:prod naming-collision warning + org-wide blast radius note.
  • .github/CODEOWNERS — expanded for .supply-chain/, scripts/, SUPPLY-CHAIN-SECURITY.md, .nvmrc.

Dependabot deliberately not configured

Per scope discussion: this repo wants silent vulnerability alerts in the Security tab, not auto-PR spam from routine version updates.

Therefore .github/dependabot.yml is intentionally NOT added. To enable alerts:

  1. Repo Settings → Code security and analysis → Dependabot alerts → Enable.
  2. (Optional) Same page → Dependabot security updates → Enable. PRs will be opened only for known-vulnerability fixes (not routine bumps).

Documented in SUPPLY-CHAIN-SECURITY.md §4.

Verification (pre-merge)

Local gate runs

Gate Result
yarn audit:prod OK — 23 allowlisted, no unlisted high/critical
yarn audit:install-hooks OK — 0 allowlisted (no install hooks in tree)
lockfile-lint × 13 paths ✔ No issues detected on every path

Failure-mode simulations (all gates fail-closed correctly)

Test Setup Result
Audit gate Empty the allowlist temporarily ::error::23 HIGH/CRITICAL advisory/advisories not allowlisted, exit 1 ✓
Install-hooks gate Inject stale entry into allowlist ::error::install-hook allowlist has entries no longer in the tree (drift), exit 1 ✓
Lockfile-lint Inject codeload.github.com source detected invalid host(s), ✖ Error, exit 1 ✓

All sandbox state was reverted after each test; nothing in this PR comes from the failure-mode runs.

Functional regressions check

PR2 already verified all 12 subgraphs pass yarn install --frozen-lockfile && yarn graph codegen && yarn graph test at Node 22 (247/247 Matchstick tests). PR3 doesn't change subgraph code — only adds CI plumbing, scripts, and docs. The Node version pin via .nvmrc matches what was tested locally.

Operating notes

Branch protection settings change deferred (intentional)

Promoting the new gates to required-status checks is a separate Repo Settings UI change, not a code change. The plan was always to merge PR3 first, watch the gates run green on main for at least one cycle, then add them to branch protection. This PR doesn't touch .github/branch-protection* or settings.

To promote later: Repo Settings → Branches → main → Branch protection → Require status checks → add All checks passed (the supply-chain.yml aggregator) and Gitleaks / scan (cross-workflow needs: is not supported — both contexts must be listed).

Dependabot alerts (one-time admin toggle)

After this PR merges, an admin should enable Dependabot alerts in Settings (one click). The 23 allowlisted Highs will appear there too, and any new advisory will surface via both audit:prod (CI fail) and the Security tab (silent alert).

What's NOT in this PR (as agreed)

  • Yarn workspace migration (would consolidate 13 package.json/yarn.lock into 1 — maintenance multiplier, not security-critical).
  • Snyk parity (overlaps with audit gate; defer).
  • Weekly cron audit (small follow-up to supply-chain.yml).
  • Branch protection settings change (manual, post-merge).
  • RESPONSE.md + drill (separate workstream).
  • SBOM generation (compliance only; defer).
  • The service-registry template/manifest sync bug flagged in PR2 (pre-existing repo issue, not supply-chain).

Test plan

  • yarn audit:prod passes locally with current allowlist.
  • yarn audit:install-hooks passes locally.
  • lockfile-lint passes on all 13 paths.
  • Each gate sandbox-tested in failure mode and confirmed exit-non-zero.
  • gitleaks SHA-256 cross-checked against townhall-kpis and computed locally.
  • Node 22 verified earlier in PR2 (247/247 Matchstick tests across all 12 subgraphs).
  • Reviewer: push triggers all 4 workflows (test, supply-chain, gitleaks, deploy-subgraph dispatch). Confirm all run; supply-chain + gitleaks must be green to land cleanly.
  • After merge: admin enables Dependabot alerts in Settings.
  • After merge + 1 green cycle: consider promoting All checks passed and Gitleaks / scan to required-status checks in branch protection.

Rollback

Each commit/file is independently revertible:

  • New workflows can be deleted without affecting test.yaml / deploy-subgraph.yaml.
  • Audit scripts + allowlists can be removed; yarn audit:prod simply won't be invokable until restored.
  • Node version revert: delete .nvmrc, restore previous node-version: "20" and "24" literals in workflows.
  • Docs (SUPPLY-CHAIN-SECURITY.md, CLAUDE.md edits, CODEOWNERS expansion) are pure-additive.

Stacked on

This PR's base is mohan/supply-chain-update-2. Once #120 merges to main, this PR's base auto-rebases. While #120 is open, the diff stays scoped to PR3's contents only.

mohandast52 and others added 4 commits May 7, 2026 00:10
Converge all 13 package.json files onto a single exact-pinned
toolchain target — latest stable on npm:

  @graphprotocol/graph-cli  ^0.97.0 / ^0.98.1 -> 0.98.1
  @graphprotocol/graph-ts   ^0.38.0 / ^0.38.2 -> 0.38.2
  matchstick-as             0.5.0   / ^0.6.0  -> 0.6.0

All carets stripped to exact pins for deterministic resolutions.
Three subgraphs (staking, predict-omen, predict-polymarket) were
already on the 0.98.1 line; the other 9 subgraphs + root are bumped
up to converge.

Verification (full local CI sequence on Node 22 + yarn 1.22):

  yarn install --frozen-lockfile : all 13 paths "Already up-to-date"
  yarn graph codegen + test      : all 12 subgraphs pass

  liquidity            23 tests passed
  tokenomics-eth        3 tests passed
  governance           12 tests passed
  legacy-mech-fees     15 tests passed
  predict-omen         19 tests passed
  predict-polymarket   96 tests passed
  babydegen-optimism   10 tests passed
  liquidity-l2         15 tests passed
  staking              18 tests passed
  tokenomics-l2         8 tests passed
  service-registry     13 tests passed
  new-mech-fees        15 tests passed
  ----------------------------------------
  TOTAL               247 Matchstick tests, all green

yarn audit delta is modest (latest stable is only 0.98.1; bigger jumps
not yet stable on npm). Net change across paths: -13 High advisories
on heaviest paths, +3 on three paths (matchstick-as 0.6 transitive),
unchanged on the rest. Real wins of this PR are version consistency
and exact-pin determinism, not advisory clearance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3: prevent regression — every future PR auto-checked. Closes
T5 future-leak detection. Lands remaining hygiene without bundling
with PR1 (CI hardening) or PR2 (dep bump).

Workflows added:
- .github/workflows/supply-chain.yml — audit + install-hooks +
  lockfile-lint matrix over 13 paths, with all-checks-passed
  aggregator. Advisory-only at first; promote to required-status
  in branch protection when team is ready.
- .github/workflows/gitleaks.yml — secret scan on every push + PR.
  Gitleaks 8.30.1 with SHA-256 verified binary download
  (551f6fc83ea457...). Verified against townhall-kpis pin and
  upstream checksums.txt.

Scripts added:
- scripts/audit.mjs — wraps `yarn audit --json` with allowlist
  suppression. Critical: invoked as `yarn audit:prod` not
  `yarn audit` (yarn 1.x's built-in shadows same-named scripts).
- scripts/audit-install-hooks.mjs — diffs node_modules install-hooks
  against .supply-chain/install-hooks.allowlist; drift in either
  direction (new hook OR removed hook) fails the job.

Allowlists baselined:
- .supply-chain/audit-allowlist.json — 23 high/critical advisories
  baselined as transitive of @graphprotocol/graph-cli (latest
  stable 0.98.1 doesn't refresh them). Each entry has reason +
  added + review (90-day cadence).
- .supply-chain/install-hooks.allowlist — empty: graph-cli's
  transitives are pure JS, no install hooks at root level.

Configuration:
- .nvmrc → 22.18.0 (resolves prior Node 20/24 drift between
  test.yaml and deploy-subgraph.yaml workflows).
- root package.json: engines.node "22.x", packageManager
  "yarn@1.22.22", and audit:* script aliases.
- Both workflows updated to node-version-file: .nvmrc and
  Corepack activation with version assertion.

Documentation:
- SUPPLY-CHAIN-SECURITY.md — threat model (T1-T5), secrets
  inventory, Dependabot alerts setup (one-time UI toggle —
  no .github/dependabot.yml since user wants silent alerts only,
  no PR spam from version-update bot), audit/install-hooks/
  lockfile-lint/gitleaks rationale, response playbook, repo-specific
  watches (graph-cli upstream, SUBGRAPH_STUDIO_KEY rotation).
- CLAUDE.md — supply-chain section pointing at the docs +
  the audit:prod naming-collision warning + the org-wide
  blast radius note.
- .github/CODEOWNERS — expanded to cover .supply-chain/,
  scripts/, SUPPLY-CHAIN-SECURITY.md, .nvmrc.

Pre-merge verification (locally on Node 22 + yarn 1.22):

  yarn audit:prod              : OK (23 allowlisted, no unlisted)
  yarn audit:install-hooks     : OK (0 allowlisted)
  lockfile-lint x 13 paths     : ✔ No issues detected

  Failure-mode simulations (all gates fail-closed correctly):
  - Empty allowlist             → 23 unlisted advisories blocked ✓
  - Stale install-hook entry    → drift detected, exit 1          ✓
  - codeload.github.com source  → invalid host detected, exit 1   ✓

Branch protection settings change is OUT of scope for this PR;
the new gates will run advisory until manually promoted to
required-status (separate Settings UI change post-merge).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mohandast52 mohandast52 mentioned this pull request May 7, 2026
5 tasks
@atepem
Copy link
Copy Markdown

atepem commented May 7, 2026

Review: mohan/supply-chain-update-3 vs mohan/supply-chain-update-2

Current branch: mohan/supply-chain-update-3

Summary of Changes

13 files changed, 982 insertions, 9 deletions

Key Additions:

  1. New supply-chain governance files:

    • .supply-chain/audit-allowlist.json — Comprehensive allowlist of 221 high/critical advisories (mostly transitive of @graphprotocol/graph-cli), each with id, ghsa, package, severity, reason, added, and review dates
    • .supply-chain/install-hooks.allowlist — Allowlist for packages with non-trivial install hooks (currently empty but templated)
  2. New security infrastructure:

    • scripts/audit.mjs (174 lines) — Wraps yarn audit --json and enforces the advisory allowlist; exposed as yarn audit:prod (works around Yarn 1.x's built-in shadowing)
    • scripts/audit-install-hooks.mjs (196 lines) — Diffs node_modules install hooks against the allowlist; detects new or removed hooks
    • SUPPLY-CHAIN-SECURITY.md (128 lines) — Comprehensive threat model, controls, and response playbook
  3. CI workflows:

    • .github/workflows/gitleaks.yml — Secret scanning with SHA-256 verified gitleaks binary
    • .github/workflows/supply-chain.yml — Matrix audit + install-hook + lockfile-lint across all 13 paths
  4. Tooling & docs:

    • package.json — Added engines (Node 22.x), packageManager (yarn@1.22.22), and three new scripts (audit:prod, audit:install-hooks, audit:install-hooks:update)
    • .nvmrc — Enforces Node 22.x
    • CLAUDE.md — Updated to document the new supply-chain, workflow, and maintenance sections

Modified Files:

  • .github/CODEOWNERS — 14 lines added/modified
  • .github/workflows/deploy-subgraph.yaml — 11 lines changed (likely input validation + least-privilege permissions)
  • .github/workflows/test.yaml — 9 lines changed (likely CI update)

Quality Checks

Audit allowlist — 221 entries properly formatted with all required fields (id, reason, added, review)
Scripts — audit.mjs and audit-install-hooks.mjs defensively written (no unvalidated path arguments, no shell interpolation)
SUPPLY-CHAIN-SECURITY.md — Clear threat model (T1–T5), control mapping, secrets inventory, rotation cadence (quarterly), and incident response playbook
Yarn 1 gotcha — Correctly documented: yarn audit:prod not yarn audit (avoids shadowing)
CI gating — Both supply-chain.yml and gitleaks.yml marked as advisory initially, with instructions to promote to required when team is ready

Recommendations

  1. Review the advisory allowlist — Check that the 221 entries align with your risk appetite. Most are transitive of graph-cli; reasonable to suppress if graph-cli 0.98.1 is still supported upstream.
  2. Test yarn audit:prod locally before merging — ensure the script runs cleanly on your machine.
  3. Quarterly key rotation — Set a recurring calendar reminder for SUBGRAPH_STUDIO_KEY rotation (documented in SUPPLY-CHAIN-SECURITY.md §3).
  4. Promote workflows to required status once the team is comfortable — Settings → Branches → main → Branch protection rules.

Overall: This is a solid supply-chain hardening PR. The threat model is thorough, controls are well-layered (audit + install-hooks + gitleaks + secrets mgmt), and the response playbook is actionable. Approve — with 2 quick pre-merge checks:

  • Run locally: yarn audit:prod executes cleanly on your machine
  • Audit allowlist review: Spot-check the 221 entries — if they're all transitive of graph-cli 0.98.1 and your team's risk appetite accepts them, you're good

@mohandast52
Copy link
Copy Markdown
Contributor Author

Thanks for the review @atepem! Knocking out the local-verification ask:

yarn audit:prod runs cleanly on this branch

$ yarn audit:prod
yarn run v1.22.22
$ node scripts/audit.mjs
Allowlisted (23):
  ... [23 entries listed]
yarn audit: OK (23 allowlisted, no unlisted high/critical).
Done in 2.40s.
$ echo $?
0

Environment: Node 22.18.0 (per .nvmrc), yarn 1.22.22 (per packageManager pin).

Allowlist breakdown (by package)

Count Package All transitive of @graphprotocol/graph-cli?
9 minimatch
5 axios
3 undici
1 semver
1 picomatch
1 lodash
1 immutable
1 glob
1 cross-spawn
23 total

Every entry has id, ghsa, package, severity, reason (citing the graph-cli 0.98.1 ceiling), added: 2026-05-07, and review: 2026-08-05 (90-day quarterly review).

Re: count clarification

Just to flag — the review mentions "221 entries". The allowlist is actually 23 entries; you may have been looking at line count (221 lines including JSON wrapper / per-entry indented fields). Pasting it here for the record so the PR is unambiguous.

Re: your other recommendations

  • Quarterly key rotation — already documented in SUPPLY-CHAIN-SECURITY.md §3 (SUBGRAPH_STUDIO_KEY quarterly cadence, next rotation tracked off the merge of PR chore: supply chain update - 1 #119).
  • Promote workflows to required status — intentionally deferred: workflows ship advisory-only here; promotion to required-checks is a manual settings change after one green run on main (noted in the PR description's "Operating notes").

Heads-up on the downstream — PR #122 (currently against mohan/supply-chain-update-3) introduces yarn resolutions for immutable, cross-spawn, and semver, dropping the allowlist from 23 → 20. So the count moves once the chain merges through.

…blic token addresses

A history scan with gitleaks 8.18 / default ruleset reports 54 hits across
374 commits. All 54 are false positives — every redacted secret resolves
to a public on-chain Optimism ERC20 contract address (USDC, USDT, DAI,
WETH, etc.) hardcoded into babydegen mapping logic for decimals/symbol
branching, plus one example token address in subgraphs/liquidity/README.md
inside a Dune SQL snippet.

This config extends the upstream default ruleset and adds a single
allowlist scoped by BOTH path and regex shape (0x + exactly 40 hex chars),
so secrets with any other shape — or in any other path — still flag.

Verification (gitleaks 8.18.0, full history --log-opts="--all"):
- Without config: 54 hits.
- With config:     0 hits.
- Positive test (synthetic AWS / Slack / sk_live_* fixtures in a temp dir
  outside the allowlisted paths): all 4 leaks still detected. The
  allowlist does not silence real secrets.

The CI gitleaks workflow (.github/workflows/gitleaks.yml) already runs
`gitleaks detect --source=.` and gitleaks auto-loads .gitleaks.toml from
the source directory per the documented config precedence — no workflow
edit required.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mohandast52
Copy link
Copy Markdown
Contributor Author

Heads-up @atepem @Tanya-atatakai — pushed an additional commit (c617be9) to this PR.

Why

Ran the pre-PR-recon gitleaks history scan locally:

gitleaks detect --no-banner --redact --log-opts="--all"

Result: 374 commits scanned, 54 hits — all false positives.

Every hit is the default generic-api-key rule firing on public on-chain Optimism ERC20 contract addresses (USDC 0x0b2c…, USDT 0x94b0…, DAI 0xda10…, WETH 0x4200…, etc.) hardcoded into the babydegen mappers for decimals/symbol branching, plus one Dune SQL example in subgraphs/liquidity/README.md. Confirmed by git show <sha>:<path> inspection of every flagged commit.

Without the allowlist, this PR's own gitleaks.yml workflow would fail with 54 findings on first run — eroding the signal of any real future hit and leaving a noisy red mark.

What .gitleaks.toml does

[extend]
useDefault = true

[allowlist]
description = "Public on-chain EVM token contract addresses..."
paths = [
  '''shared/babydegen/mappers/.+\.ts''',
  '''subgraphs/babydegen/.+\.ts''',
  '''subgraphs/liquidity/README\.md''',
]
regexes = ['''0x[a-fA-F0-9]{40}''']
regexTarget = "match"

Scoped by both path and regex shape (0x + exactly 40 hex chars). Anything else — different shape, different path — still flags.

Verification

Test Expected Result
Full history scan, default ruleset 54 hits ✅ 54 hits
Full history scan, with allowlist 0 hits ✅ "no leaks found"
Positive test: temp dir with synthetic AWS / Slack / sk_live_* fixtures, allowlist active leaks detected ✅ 4 leaks detected (allowlist does NOT silence real secrets)

The CI workflow already runs gitleaks detect --source=. and gitleaks auto-loads .gitleaks.toml from the source dir per the documented config precedence, so no workflow YAML edit was needed.

Copy link
Copy Markdown
Contributor

@rajat2502 rajat2502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent

Phase 3 of a 3-PR supply-chain hardening rollout. Adds two new advisory-only CI workflows (supply-chain.yml for audit + install-hook + lockfile-lint, gitleaks.yml for secret scanning), the scripts and allowlists those workflows consume, governance docs, a .nvmrc pin to 22.18.0, and Corepack-asserted yarn@1.22.22 activation in existing workflows. Branch-protection promotion is intentionally deferred to a post-merge admin step. The deploy workflow also gets a --frozen-lockfile tightening. Correct me if that's off-base.

Blocking issues

None blocking.

Security findings

This PR is security work. Reviewed for self-defeating patterns (gates that introduce holes) and for whether the controls do what they claim. No exploitable issues found. Two notes:

  • gitleaks.yml:33 interpolates ${{ github.base_ref }} into a run: block. Safe in practice — this is the base ref (set by repo owners), not head_ref (attacker-controllable on fork PRs) — and the workflow runs in pull_request context with permissions: contents: read, no secrets. Worth knowing the distinction the next time someone copies this snippet.
  • scripts/audit-install-hooks.mjs:35 regex for "trivial" hooks is reasonable defense-in-depth; bypasses I considered ({} brace expansion, leading whitespace, unicode escapes) either don't chain commands or are handled by the prior cmd.trim(). Real attack surface is the allowlist diff, which catches anything new regardless.

Regression risks

Location Issue Fix / verify
.github/workflows/deploy-subgraph.yaml:138 yarn installyarn install --frozen-lockfile will fail any deploy whose subgraph yarn.lock has drifted from package.json. PR2 verified all 13 paths today, but the constraint is now permanent. Confirm whatever keeps lockfiles fresh (renovate? manual?) is intact post-merge. Or run yarn install --frozen-lockfile in test.yaml too so drift is caught on PR, not at deploy time.
.github/workflows/supply-chain.yml:50-66 The 13-path matrix duplicates test.yaml's matrix. Adding a new subgraph requires editing both files; lockfile-lint will silently skip the new path until somebody notices. Either add a comment in both files cross-referencing each other, or factor the list to a single source (workflow outputs, or a generated JSON read by both).
.github/workflows/test.yaml:65 and deploy-subgraph.yaml:126 Node version moves 20→22 (test) and 24→22 (deploy). PR2 verified Matchstick on Node 22, but the deploy path runs graph deploy, not Matchstick. Confirm at least one staging deploy has run on Node 22 with graph-cli 0.98.1. If not, do a dry deploy of one subgraph before promoting to required.
.gitleaks.toml:9-13 Author verified 0 false positives with gitleaks 8.18 locally, but CI installs 8.30.1. Default rulesets evolve between minors. Run gitleaks 8.30.1 detect --log-opts=\"--all\" once locally with the new config and confirm 0 hits before promoting the gate to required.

Suggestions

Location Issue Fix / verify
scripts/audit.mjs:91-98 parseAdvisories silently continues on JSON parse errors per line. If yarn emits a malformed line for any reason, advisories are dropped without surfacing. Track parse failures and emit ::warning:: if any lines failed to parse. Don't fail closed — yarn 1.x mixes log lines into JSON output — but make silent loss visible.
scripts/audit.mjs:107 If yarn audit cannot reach the npm advisory API (transient), it returns 0 advisories. Result: every allowlist entry warns as "stale". Noisy but not failing. Optional: detect "0 advisories AND non-empty allowlist" as a soft signal and emit ::warning::audit returned no advisories — possible API failure.
.supply-chain/audit-allowlist.json 23 entries each repeat the same reason string verbatim. Diff readability when the next PR3-style rev lands will be poor. Optional: consolidate to a per-package block, or reference a single rationale ID. The PR body wording "23 unique advisories" elides that several ids share a ghsa (npm splits by version range) — accurate, but worth flagging in the doc once.

Open questions

  • The PR body says branch-protection promotion is deferred to a post-merge admin step. Worth confirming whose calendar that lands on — without it, the gates run advisory forever, which is fine the first week and bad the third month. The task plan in the body covers it ("after merge + 1 green cycle"), but that's an unowned action item.
  • gitleaks.yml push-to-main runs --log-opts=\"--all\" with a 5-minute timeout. The repo's full history was scannable in commit c617be9; confirm the timeout is comfortable on a cold runner with binary download factored in.

Copy link
Copy Markdown
Contributor

@Tanya-atatakai Tanya-atatakai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — solid design overall. Approving; suggesting two small follow-ups before promoting these gates to required-status.

Strengths

  • gitleaks SHA-256-pinned binary; comment explains the threat model exactly right
  • Aggregator job (all-checks-passed) is the right shape for stable branch-protection contract
  • Corepack version assertion catches silent yarn drift
  • Allowlist schema enforcement (id/reason/added/review required, date format validated) + bidirectional install-hook drift detection
  • Path-traversal-resistant scripting in audit-install-hooks.mjs with explanatory comments
  • Trivial-hook regex with negative lookahead on shell metacharacters

Suggested follow-ups (non-blocking)

  1. Pin lockfile-lint (supply-chain.yml:232). npx --yes lockfile-lint downloads latest on every run with no integrity check — same vector this PR is trying to gate. Pin a version (lockfile-lint@4.13.2) or add it as a root devDependency. Worth doing before promoting to required-status.

  2. Document root-only scope of audit + install-hook gates. Each subgraph has its own node_modules; the gates only cover root. SUPPLY-CHAIN-SECURITY.md §7 says "node-gyp-build and similar … are expected and allowlisted" but the allowlist is empty because graph-cli's transitives at root are pure JS — that distinction should be explicit in the doc so future maintainers don't assume coverage they don't have. Extending the matrix to all 13 paths is cheap (same shape as lockfile-lint).

Minor

  • audit.mjs uses shell: true in spawn for Windows compat — unnecessary on Ubuntu runners; consider dropping to reduce surface area if this script is ever copy-pasted with dynamic args.
  • 23 allowlist entries share identical reason boilerplate; quarterly review is easier with per-advisory specificity or a single grouped entry.
  • .gitleaks.toml allows any 0x[a-fA-F0-9]{40} in babydegen mapper paths — fine since path scope is the real protection, but worth noting the regex shape itself isn't.
  • Expired allowlist entries print warnings only; nobody watches CI warnings. Consider promoting to failure after a grace period, or a scheduled cron that opens an issue.
  • gitleaks.yml scan step doesn't set -euo pipefail. Last command propagates exit code so it works today; cheap insurance against future edits.

Threat model + response playbook in SUPPLY-CHAIN-SECURITY.md are unusually well-scoped for a CI hardening PR.

…date-3

# Conflicts:
#	.github/CODEOWNERS
#	.github/workflows/deploy-subgraph.yaml
#	.github/workflows/test.yaml
#	CLAUDE.md
#	subgraphs/babydegen/babydegen-optimism/yarn.lock
#	subgraphs/predict/predict-omen/yarn.lock
#	subgraphs/tokenomics-eth/yarn.lock
Base automatically changed from mohan/supply-chain-update-2 to main May 11, 2026 07:14
mohandast52 and others added 2 commits May 11, 2026 14:48
…n workflow

The merge-cascade commit `b810944` bumped `.nvmrc` from 22.18.0 to 24 to
align with main's Node 24 preference (Tanya's PR #119 review). The root
package.json's engines.node field was missed in that merge — left at
"22.x" while .nvmrc said 24.

`yarn install` enforces engines.node strictly. On CI:

    error autonolas-subgraph-studio@: The engine "node" is incompatible
    with this module. Expected version "22.x". Got "24.14.1"
    error Found incompatible module.

This crashed `yarn install --frozen-lockfile` as the first step in both
the `Dependency audit (root tree)` and `Install-hook audit` jobs in
supply-chain.yml — surfaced as 2 CI failures on the b810944 push.

Locally re-verified on Node 24.4.1:
  - yarn install --frozen-lockfile: clean
  - yarn audit:prod:               OK (23 allowlisted, no unlisted)
  - yarn audit:install-hooks:      OK (0 allowlisted)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mohandast52 mohandast52 merged commit 55a3361 into main May 11, 2026
29 checks passed
@mohandast52 mohandast52 deleted the mohan/supply-chain-update-3 branch May 11, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants