fix(deps): re-resolve browserslist to clear two HIGH OSV advisories (#845) - #844
Conversation
`security:deps` (osv-scanner) started failing on every PR in the repo today: GHSA-73wf-gq98-2v4g 7.5 npm browserslist 4.28.5 fixed in 4.28.7 GHSA-c83g-rgw3-j3cx 7.5 npm browserslist 4.28.5 fixed in 4.28.7 Not specific to any branch — `main` carries 4.28.5, so the gate fails wherever it runs. Reproduced locally against `main` before and after this change. Lockfile-only, following the #637 precedent. `browserslist` is transitive (no direct entry in any `package.json`), so `yarn upgrade browserslist` is a no-op under yarn v1; dropping the pinned stanza and re-installing lets it resolve within the existing `^4.24.0` range, landing 4.28.8. No `package.json` change, so no `resolutions` entry and nothing to mirror into `integrations/jira-forge-app` — the advisory's only source is `yarn.lock`. The 56-line delta is confined to browserslist and the five data packages it pins: baseline-browser-mapping, caniuse-lite, electron-to-chromium, node-releases, update-browserslist-db. Verified: `osv-scanner` reports "No issues found", and `mise run build` is green including the Astro docs build, which is what actually consumes browserslist.
ayushtr-aws
left a comment
There was a problem hiding this comment.
PR #844 Review — chore(deps): re-resolve browserslist to clear two HIGH OSV advisories
- Verdict
Approve with a process nit. This is a correct, minimal, well-justified lockfile-only hotfix that unblocks security:deps
repo-wide. The only issue is governance (no linked approved issue / branch lacks an issue number), which I'd resolve in
parallel rather than block on given main is currently red.
- Vision alignment
Directly serves "bounded, reliable, improvable control plane." A HIGH-severity advisory failing the security:deps gate on main
blocks every PR in the repo — this restores the merge path with the smallest possible surface. No tenet trade-off; no ADR
required.
- Blocking issues
None on the code/lockfile itself. Verified:
- browserslist is genuinely transitive — no direct entry in any package.json, and no resolutions entry (root resolutions block
confirmed: browserslist absent). So the change is pure yarn.lock, and the AGENTS.md transitive-pin rule (mirror into
integrations/jira-forge-app/overrides) correctly does not apply — that project's overrides are brace-expansion/fast-uri/undici
only, and it doesn't consume browserslist. ✅ - 4.28.8 satisfies the existing ^4.24.0 range and is > the advisory fix (4.28.7). ✅
- Single browserslist@^4.24.0 lock entry — browserslist is the sole consumer of the five bumped data packages, so no
conflicting/stale second entries are left behind after merge. ✅ - CI security-pr (osv-scanner) and build (agentcore) both green — osv-scanner is the authoritative check for this exact
change. ✅
-
Non-blocking suggestions / nits
-
No linked/approved issue; branch omits the issue number (chore/browserslist-osv). Per ADR-003 and the branch-naming
convention, this is unauthorized-work shape. The cited #637 precedent actually had an issue (branch fix/636-osv-deps-bump).
Recommend: file a P1 tracking issue for the advisory and reference it, so the audit trail matches the precedent. Given the
repo-wide breakage, I wouldn't hold merge on this — but it should be filed, not skipped. -
Because there's no resolutions pin, a future forced re-resolution could float browserslist again. That's normal
transitive-dep behavior (the lockfile pins the exact version for all installs), so no pin is warranted here — just noting the
residual for awareness. -
Documentation
No docs impact. No behavior, contract, env var, command, guide, or docs/design/docs/guides prose changed → no Starlight mirror
sync needed. Correctly none included.
- Tests & CI
No test changes appropriate for a lockfile bump. All required checks passing (CodeQL ×3, build agentcore, security-pr,
PR-title lint, dead-code advisory). Bootstrap synth-coverage: not applicable — no CDK construct/stack/handler or CFN
resource-type changes.
- Review agents run
This diff touches only yarn.lock — no TypeScript/Python source, types, comments, tests, error-handling, IAM, Cedar, network,
secrets, or input-gateway code. Accordingly:
- code-reviewer — omitted: no source/style to review.
- silent-failure-hunter — omitted: no error-handling/fallback code.
- type-design-analyzer — omitted: no new/changed types.
- comment-analyzer — omitted: no comments changed.
- pr-test-analyzer — omitted: a lockfile version bump has no unit-testable behavior; validation is the osv-scanner gate.
- /security-review — omitted as a code review: its scope (IAM/Cedar/network/secrets/input-gateway) isn't touched; the
security-relevant check for a dependency bump is osv-scanner, which ran and passed in CI (security-pr) and which I
reproduced-equivalent by confirming 4.28.8 ≥ 4.28.7 fix version.
(Note: the pr-review-toolkit agents aren't registered as invokable agent types in this environment; the above maps to the
toolkit's scopes.)
- Human heuristics
- Proportionality — ✅ Pass. 28/28 lines, one file; matches the problem exactly. No new abstraction.
- Coherence — ✅ Pass. Follows the established #637 re-resolve pattern; belongs in the lockfile.
- Clarity — ✅ Pass. Excellent PR description: names the advisories, explains why it's repo-wide not branch-specific, and why
lockfile-only (transitive, yarn-v1 no-op). - Appropriateness —
⚠️ Minor. Change itself is maintainable and correct; the only gap is the missing issue/branch-number
governance (nit #1).
Bottom line: merge-ready on the code. File the P1 tracking issue to close the ADR-003 loop.
|
Thanks — governance nit taken, and you were right that the #637 precedent had an issue behind it (#636, branch I re-verified the facts before writing the issue rather than copying them out of the PR body: The branch name is the one part I have not changed. Renaming an approved, green PR's head branch would disrupt it for a naming convention, so I would rather leave the audit trail in the issue link, the title and the commit than churn the ref while On your second nit — recorded in #845 under Notes rather than acted on, since I agree no |
Closes #845.
What
Re-resolves
browserslistinyarn.lockfrom 4.28.5 to 4.28.8 to clear two HIGH OSV advisories.Why this is blocking everything, not one branch
security:depsbegan failing across the repo today:mainitself carries 4.28.5, so the gate fails wherever it runs. Confirmed by reproducing it locally againstmain, and visible in the timing: #831'ssecurity-prpassed at 13:42 and #681's failed at 17:28 on the same lockfile state. Dependabot #838 does not cover it — that PR was last updated at 13:28, before the advisory, and its diff does not touch browserslist.Why lockfile-only
Following the #637 precedent.
browserslistis transitive — no direct entry in anypackage.json— soyarn upgrade browserslistis a no-op under yarn v1. Dropping the pinned stanza and re-installing lets it resolve within the existing^4.24.0range, which lands 4.28.8.That keeps this a pure lockfile change: no
package.jsonedit, so noresolutionsentry and nothing to mirror intointegrations/jira-forge-app(per the AGENTS.md transitive-pin rule — the advisory's only source isyarn.lock).Scope
56 lines, confined to browserslist and the five data packages it pins:
baseline-browser-mapping,caniuse-lite,electron-to-chromium,node-releases,update-browserslist-db. No other package moved.Verification
mise run security:deps→ No issues found (was: 1 package, 2 HIGH)mise run buildgreen — including the Astro docs build, which is the actual consumer of browserslist