MetaMask / MetaMask/metamask-extension

[P2] Finish postcss remediation: 8.5.23 floor, last two postcss 7 paths, remaining LavaMoat policies

Open
#44,866 1 comment 0 reactions 0 assignees View on GitHub
area-buildSystem ta-triaged type-security
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

### What is this about?

Follow-up to #44865, which clears both published postcss advisories and removes the stylelint-owned postcss 7 subtree. Three things were deliberately left out of that PR, plus one open question about whether these advisories should block releases at all.

- **The postcss floor lands on 8.5.21, and 8.5.23 exists.** 8.5.23 shipped 2026-07-24 17:05 UTC with *"Do not load source map without `opts.from` for security reasons"* — a **third** hardening of the same `sourceMappingURL` surface after 8.5.12 ([GHSA-6g55-p6wh-862q](https://github.com/advisories/GHSA-6g55-p6wh-862q)) and 8.5.18 ([GHSA-r28c-9q8g-f849](https://github.com/advisories/GHSA-r28c-9q8g-f849)). No advisory is published for it yet.
- The 8.5.18 fix anchors its restriction on `opts.from`, but GHSA-6g55 states the bug triggers under **default options with no `from`** — so 8.5.18 plausibly leaves the no-`from` path open, and a third GHSA covering ≤ 8.5.22 is likely.
- `npmMinimalAgeGate: 4320` in `.yarnrc.yml` blocks 8.5.23 until **2026-07-27 17:05 UTC**, which is why #44865 ships 8.5.21.
- **Two postcss 7 descriptors remain**, both devDependencies and build-time only — and they are two *different* tasks, not one removal:
- `resolve-url-loader@3.1.5` owns `postcss@7.0.36` (exact pin). **Upgradeable to v8:** `resolve-url-loader@5.0.0` moves to `postcss@^8.2.14`. It is a direct devDependency at `^3.1.5`, so this is a two-major loader bump (the loader's build API changed across 3→5) and needs a build run to confirm.
- `gulp-sourcemaps@3.0.0` owns `postcss@^7.0.16` via `@gulp-sourcemaps/identity-map@2.0.1` — **no v8 path in place:** identity-map's latest release (2.0.1) hard-pins postcss `^7` with no successor, and `gulp-sourcemaps@3.0.0` is itself already latest. Clearing this one means removing `gulp-sourcemaps`, patching identity-map locally, or a scoped force-resolve (risky — it hard-deps `^7`).
- Verified against postcss 7.0.39's `lib/previous-map.js`: its `loadMap` annotation branch reads an annotation-derived path with no `.map`-extension gate (the 8.5.12 fix) and no path-traversal guard (the 8.5.18 fix), so both advisory vectors are present in code — but exposure is build-time only, as neither consumer ships to users.
- **Only the webpack *build* policy was regenerated, and it was regenerated the wrong way.** `validate-lavamoat-policies.yml` runs three separate jobs — `webpack:lavamoat:policy:build`, `:mv2` and `:mv3` — so the MV2 and MV3 policy sets are validated in CI too, and both are still stale on the branch. Policy regeneration is owned by `@metamaskbot update-policies`, not by running `lavamoat:*` locally and committing the result.

### Scenario

N/A — technical task, not a user story.

### Design

N/A — no user-facing surface.

### Technical Details

#### 1. Raise the postcss floor to `^8.5.23`

- Bump after the age gate opens on 2026-07-27, then `yarn dedupe nanoid` if a second `nanoid` copy reappears.
- **Verify rather than assume.** 8.5.19–8.5.22 were parser regressions *caused by* the security patches — lost semicolons before comments, missing space after setting `AtRule#params`, bad `before` cleaning on new `Root` nodes, AST errors on warnings. Re-run `yarn lint:styles` and `yarn webpack:lavamoat:build` after bumping.
- **`yarn webpack:lavamoat:build` exits 0 even when it fails.** A policy violation surfaces as `ERROR in ../ui/css/index.scss` / `not in allowlist` in the log with a zero exit code. Grep the log for those strings; the exit code is not a usable pass signal.

#### 2. Clear the last two postcss 7 paths

- **`resolve-url-loader` 3.1.5 → 5.x.** Sole consumer is `development/webpack/webpack.integration.tests.config.ts:84`. Also touch the `lavamoat/build-system/policy.json` entry and the `resolve-url-loader>es6-iterator>es5-ext` allowlist entry in `package.json`. v5 needs postcss 8 and sourcemaps from `sass-loader`.
- **`gulp-sourcemaps`** — sole consumer on main is `development/ts-migration-dashboard/scripts/build-app.ts:98,105` (`sourcemaps.init()` / `sourcemaps.write()` in the dashboard stylesheet pipeline), and it is patched via `.yarn/patches/gulp-sourcemaps-npm-3.0.0-1ae0fbef6d.patch`. With no upgrade available, the options are:
- drop sourcemaps from the dashboard stylesheet build,
- replace `gulp-sourcemaps`, or
- scope an audit exclusion, given it is dev-only.

#### 3. Regenerate the remaining LavaMoat policies

- Post `@metamaskbot update-policies` on the PR and let the bot push the policy commit. Do **not** run `lavamoat:*` locally and commit the output — that path only covers whichever policy reproduces on the dev box and silently leaves the other two sets stale.
- The bot declines while CI is still in flight (*"LavaMoat validation is still running. Please retry `@metamaskbot update-policies` after CI validation completes."*), so it has to be re-posted once checks settle.

#### 4. Confirm whether these advisories are release-blocking at all

- A lockfile walk from the **238 production dependencies** (1621 descriptors) reaches postcss **zero** times; from **devDependencies** (287 roots, 4793 descriptors) it reaches it **16** times. Every postcss-adjacent root dep is a devDependency.
- `.github/scripts/yarn-audit-and-triage.mts` takes yarn's production output at face value, so if the top-level seed set is wrong, the release block is spurious and belongs in the audit config rather than the dependency tree.
- Settle it with `yarn npm audit --recursive --environment production --json | grep postcss` on main.

#### 5. Remove the temporary release-branch audit ignores after cp

`release/13.42.0` (on postcss `^8.5.10`) temporarily ignores `1124252` (GHSA-6g55-p6wh-862q) and `1124288` (GHSA-r28c-9q8g-f849) in `.yarnrc.yml` under "postcss vulnerability", added to unblock the RC. Once #44865 cherry-picks in (postcss → 8.5.21), those advisories no longer fire — remove both ignore entries.

- **Trigger:** after #44865 lands on `release/13.42.0`. Removing them earlier re-breaks the RC audit (the branch is still on the vulnerable 8.x until then).

### Threat Modeling Framework

- **What are we working on?** Removing the remaining postcss versions carrying known arbitrary-file-read and path-traversal defects from the dependency tree, and confirming the residual exposure is understood rather than assumed.
- **What can go wrong?** The 8.5.18 fix may be incomplete for the no-`opts.from` case, leaving a file-read primitive reachable from any pipeline that runs untrusted CSS. Separately, the postcss 7 copies have no patch at all and never will.
- **What are we going to do about it?** Move to 8.5.23 once installable, and remove the two postcss 7 roots rather than relying on their unreachability.
- **Did we do a good job?** Measured by `yarn.lock` containing zero postcss 7 entries and the floor sitting at a version with all three source-map hardenings, verified by a green `lint:styles` and LavaMoat build rather than by version number alone.

### Acceptance Criteria

- [ ] `postcss` floor is `^8.5.23` and `yarn.lock` resolves to 8.5.23 or later
- [ ] `yarn lint:styles` passes and `yarn webpack:lavamoat:build` produces zero `ERROR in` / `not in allowlist` hits after the bump (checked by grepping the log, not by exit code)
- [ ] `yarn.lock` contains zero `postcss@npm:7.x` entries
- [ ] All three LavaMoat policy sets (`build`, `mv2`, `mv3`) regenerated via `@metamaskbot update-policies`, with all three `validate-lavamoat-policies` jobs green
- [ ] Production-vs-dev classification of the postcss advisories confirmed and recorded

### References

- Depends on: #44865
- Parent audit tracking issue: #44788
- Superseded attempt: #44860
- [GHSA-6g55-p6wh-862q](https://github.com/advisories/GHSA-6g55-p6wh-862q) — arbitrary file read via `sourceMappingURL` (≤ 8.5.11)
- [GHSA-r28c-9q8g-f849](https://github.com/advisories/GHSA-r28c-9q8g-f849) — path traversal in previous source map auto-loading (≤ 8.5.17)
- [postcss CHANGELOG](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) — 8.5.12, 8.5.18 and 8.5.23 entries

Contributor guide

Open the contributing guide

Research direction

Start with yarn.lock, .yarnrc.yml, development/webpack/webpack.integration.tests.config.ts, and development/ts-migration-dashboard/scripts/build-app.ts to map the remaining postcss 7 consumers and release-branch ignores. Run the listed lint, LavaMoat, and production audit commands, checking logs for policy errors rather than relying on exit codes. Done means no postcss 7 entries, postcss at 8.5.23 or later, all three policy jobs green, and the production-vs-dev audit classification recorded.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, sass, typescript, webpack
Domain
build-system, ci-cd, devops, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.