feat(security): add supply-chain-audit and lavamoat-policy-diligence skills - #83
feat(security): add supply-chain-audit and lavamoat-policy-diligence skills#83MajorLift wants to merge 4 commits into
supply-chain-audit and lavamoat-policy-diligence skills#83Conversation
…oat-policy-diligence Two composing skills for "is this dependency change safe to take". `supply-chain-audit` is the breadth pass: Socket findings, `yarn npm audit` advisories, lockfile and manifest diffs, and the fronts no upstream scanner sees because they are things the repo does to its dependencies afterwards — yarn patches that modify dependency source at install, `resolutions` that force or stub versions, `npmAuditIgnoreAdvisories` suppression lists, CI actions riding mutable tags, and yarn plugins that execute at install. `lavamoat-policy-diligence` is the depth pass it delegates capability containment to. Because a LavaMoat policy is generated from a real run, every grant has a call site by construction — so "each addition is justified" is a tautology, not a finding. It instead reads each grant's use at the installed version to find its gate, and sorts into removable / removable-at-a-cost / load-bearing. Neither renders an accept/reject verdict; disposition belongs to the people who own the dependency. Adds a CODEOWNERS entry for the new domain, defaulted to the platform teams.
supply-chain-audit and lavamoat-policy-diligence skills
Context budgetWhat this PR costs an agent, measured from an install rather than read from the diff. Three tiers, and only the first is unavoidable.
Frontmatter is the only tier paid unconditionally — every agent loads it on every run once the skill is installed, used or not, because it is what the agent reads to decide relevance. The 28 skills across the eleven open skill PRs sit at a median of ~1,716 tokens selected and ~1,860 with references followed. All are within the 1,536-character description budget. Selected is paid only when the agent picks the skill. + refs & knowledge is the ceiling if every bundled reference is then read; it is a worst case, not an expectation. Method
These figures are pinned to the commit above and drift on every push; #96 tracks automating them. |
`pr-validate` was renamed to `evidence`; both skills still named the old one, in a section heading, prose, and a `## Related` entry. `supply-chain-audit` now also links its evidence category in the catalog rather than naming it bare.
…rants Adds `--override`: overrides are where containment is widened by a person rather than observed by the toolchain, and the decision persists across regenerations, so it outlives the reason for it. On the extension's mv3/main policy: 42 tightened, 83 persisting, 9 narrowable, 2 write. Three things came from reading lavamoat-core/src/mergePolicy.js rather than inferring them, each of which had been wrong: - The effective policy is mergePolicy(generated, override). The two files are DESIGNED not to align — the generated one is regenerated on dependency updates while the override persists — so an override entry absent from the generated policy is the normal case. Calling that "never observed" was alarmist and wrong; 83 of 87 entries are in that state by construction. - `validateHierarchy` throws when both `X` and `X.y` are present, so the first version of the narrowing suggestion would have produced a policy that fails to build. The documented form denies the parent: `"X": false, "X.y": true`. - Escalation flagged every global in any package matching a name hint, labelling `Array` and `Object` as "critical class". Intrinsics are now excluded and the reason string is true of the row it appears on — an escalation list that is mostly noise trains its reader to skip it. Critical grants and write access get RAISE WITH A HUMAN and no verdict. Correctness there depends on intent and threat model, neither of which is in the policy files, and an audit that silently resolves them has substituted a guess for the thing it was asked to check.
A single escalation list mixes two different questions. A capability granted to a package the base policy did not contain arrives because the package arrived — the question there is whether the package belongs in the bundle. A capability newly granted to a package already contained is somebody's decision about that capability. Mixed together on one mv3 policy, 14 of the 24 rows were the first kind, and a list that is mostly not actionable teaches its reader to skim. Write access is exempt from the row cap: it is the smallest and highest-signal category, and truncating it hides the row that most needed a reader.
Adds a
securitydomain with two skills that compose into one question: is this dependency change safe to take?supply-chain-audit— the breadth passRuns every detector that answers a different part of that question, and treats disposition as the deliverable rather than detection (detection belongs to the tools):
yarn npm auditand advisories — known vulnerabilitiesThe part that earns it a place: the fronts no upstream scanner sees, because they are things your own repo does to dependencies after they arrive —
patches/applied at installresolutionsforcing or stubbing versionsnpmAuditIgnoreAdvisories@v4is a moving target; the SHA that runs tomorrow is not today'sThe falsifier is a lane whose finding is unaccounted for — a flagged package, an unresolved advisory, or a grant with no call site.
lavamoat-policy-diligence— the depth passCapability containment, delegated from the lane above.
The framing that makes it useful: a LavaMoat policy is generated from a real run, so every grant has a call site by construction. That makes "each addition is justified" a tautology, and therefore not a deliverable. What is a deliverable is each grant's gate — a config flag nobody sets, an API nobody calls, a branch our payloads never take, an error-only path — read at the installed version, sorted into removable / removable-at-a-cost / load-bearing, with the removal test (drop it, rebuild, run e2e) proposed for the policy owners to run.
Ships
scripts/policy-audit.py, which turns a base/head policy pair into a per-grant worklist.Neither skill renders an accept/reject verdict. That call is the reviewer's, and the disposition belongs to the people who own the dependency.
Files
domains/security/skills/{supply-chain-audit,lavamoat-policy-diligence}/. Both experimental; norepos/overlays. Commits GPG-signed.Showcase — what
lavamoat-policy-diligenceproducesextension#42867 —
@sentry/browser8.33.1 → 10.38.0. The bump added grants across the@sentry/*subtree. A reviewer asked what two of them were for. Each was answered with the upstream line, pinned to the installed version:WebAssembly→isWebAssemblyException, which runs on every exception —eventbuilder.ts#L163-L168@ 10.38.0importScripts→ main-thread detection at module scope —profiling/utils.ts#L33-L34@ 10.38.0Both sit on unconditional paths — the exception path and module scope — and under scuttling the read itself throws unless excepted. So both are load-bearing with no gate to close. That is the useful output: not "each grant has a reason" but "neither is removable, and here is the unconditional path that makes it so."
The counter-example is the reason the skill exists. A first pass on extension#45024 led with "11 additions, 11 reasons, each resolving to a line." That is tautological: the policy is generated from a real run, so every grant has a call site by construction and the count was guaranteed before anyone looked. Reading for gates — a config flag nobody sets, an API nobody calls, a branch our payloads never take — is what separates a removable grant from a load-bearing one.
Showcase — what
supply-chain-auditproducesextension#44865 — clear the postcss advisories, which started as a
yarn npm auditreport of newly blocking advisories onmain.Detection was the easy half: four in-range advisories, two of them published against
postcss8 — GHSA-6g55-p6wh-862q (arbitrary file read via attacker-controlledsourceMappingURL, ≤ 8.5.11) and GHSA-r28c-9q8g-f849 (path traversal in source-map auto-loading, ≤ 8.5.17). What the audit could not say is what to do about eleven postcss-7 paths that no upstream bump reaches.The disposition is the deliverable:
stylelint13.6.1 owned 8 of the 11 postcss-7 paths; upgrading it to 17.14.1 removed them at the source.7.0.36underresolve-url-loader,7.0.39undergulp-sourcemaps— carry backports of all four advisory fixes in.yarn/patches, scoped so the v8 tree stays on 8.5.21. This is the lane no upstream scanner sees: Socket andauditexamine a dependency as published, not as your repo patches it at install.resolutions.postcss. Falsifier: does anypostcss-7 consumer declarepostcssas a peer rather than a hard dependency? All five declare a hard^7—postcss-scss@2.1.1,postcss-less@3.1.4,sugarss@2.0.0,postcss-safe-parser@4.0.2,autoprefixer@9.8.x— so a blanket resolution would force them onto v8 and break them. That PR was closed.resolutionsis needed at all. Falsifier: any^8descriptor resolving to a second copy. All nine map to onepostcss@npm:8.5.21entry in the committed lockfile.Every claim carries the observation that would have refuted it. That is the shape the skill asks for: the tools produce the worklist, and the audit produces the disposition and the reason.
Notes for reviewers
/domains/security/line to.github/CODEOWNERS. I defaulted it to@MetaMask/extension-platform @MetaMask/mobile-platform(matching the*fallback) — please reassign if a security-owning team should hold it instead. Worth noting the existinganalytics(feat(analytics): add Sentry, MetaMetrics, Segment, and Tempo skills #76) andplatform(feat(platform): add Extension runtime architecture, debugging skills #44) domain PRs don't add CODEOWNERS entries; those are separate gaps.lint-skill-entrystructural validator #47. Written for skill-discovery matching: both skills sit next to adjacent ones (supply-chain-auditbesidelavamoat-policy-diligence), so the trigger cues are what keep a request from landing on the wrong one.Validation runs
Trial runs of this PR's skills against merged
metamask-extensionPRs nobody flagged. Every claim was re-verified against the real diff before posting. Clean results are included on purpose — a skill that only ever reports problems cannot be calibrated.supply-chain-audit@sentry/browsergainsfetch,importScripts,WebAssemblylavamoat-policy-diligencewritesupply-chain-auditsass-embeddedbump grantsworker_threads+fssupply-chain-audittar; the policy delta comes fromstreamxEach comment carries a trial-run disclaimer and links back here for feedback.