Skip to content

Commit 02b124e

Browse files
authored
fix(security): patch fast-uri, postcss, and brace-expansion advisories (#1510)
* fix(security): patch fast-uri, postcss, and brace-expansion advisories Resolve the two open Dependabot alerts plus a third high-severity advisory the repo's own audit surfaces but Dependabot had not filed, all via version-ranged pnpm overrides (they lapse once the upstream tree moves past them): - fast-uri 3.1.4 -> 3.1.5 (website): GHSA-7p8r-x3mc-p8w7, high. Host confusion via backslash authority introducer. Pulled in transitively by ajv@8.18.0; bounded to ^3.1.5 so it stays on the 3.x line ajv expects. - postcss 8.5.22 -> 8.5.25 (root): GHSA-fxqj-rqcc-2cmp, moderate. Arbitrary .map file read via attacker-controlled sourceMappingURL. Pulled in by vite (dev/test tooling). - brace-expansion 5.0.8 -> 5.0.9 (website): GHSA-rgw5-rvv9-x895, high. DoS via unbounded recursion. The existing override capped at >=5.0.8, and 5.0.8 is itself vulnerable under this newer advisory; the root already resolved to 5.0.9. Root and website audits are clean at --audit-level high (and any-severity for the website). Full test suite: 3662 passing. * harden(security): bound overrides, scope release perms, add website lockfile drift check, document archive TOCTOU intent Hardening pass over the security fixes, from a parallel review of the dependency, CI, archive, and adjacent-code surfaces. Each item is low-risk and verified; resolved dependency versions are unchanged. - deps: bound the three security overrides to their current major (brace-expansion ">=5.0.9 <6", postcss ">=8.5.23 <9"). A bare ">=X" pin would take a future major on the next lockfile regen without review; the website already models the caret-bounded idiom. - ci: scope release-prepare.yml permissions per job. The top-level block dropped "pull-requests: write"; only the "prepare" job (which opens the Version Packages PR) now holds it. The "beta" job only tags/releases and publishes via OIDC, so it inherits the narrower default (least privilege). - ci: add a "Website Lockfile Drift" job to security.yml. The website keeps its own lockfile and is never installed in CI, so a website override that stops resolving would go unnoticed and `pnpm audit` would scan a stale graph. A `pnpm install --frozen-lockfile --ignore-scripts --dir website` fails fast on that drift (root drift is already caught in ci.yml). - archive: add intent comments at the 7 js/file-system-race sites in src/core/archive.ts. The stat->read->re-stat pattern is a deliberate concurrent-change detector; the comments record why, so no future refactor (human or scanner-driven) collapses it to fd I/O and blinds the guard. Verified: 3662 tests pass, build clean, website build clean, root+website audits clean at --audit-level high, and the new frozen-lockfile check passes locally. * chore(nix): refresh pnpmDeps hash for the lockfile change The root pnpm-lock.yaml changed (postcss + brace-expansion overrides), which stales the fixed-output pnpmDeps hash and fails Nix Flake Validation. Repin to the value CI computed from the new lockfile.
1 parent 3d0701f commit 02b124e

8 files changed

Lines changed: 78 additions & 18 deletions

File tree

.github/workflows/release-prepare.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ on:
55
branches: [main]
66
workflow_dispatch: # manually cut a beta prerelease from main
77

8+
# Floor for both jobs. The prepare job widens this to pull-requests: write for
9+
# the Version Packages PR; the beta job only tags/releases + publishes via OIDC
10+
# and needs no PR access, so it inherits this narrower default.
811
permissions:
912
contents: write
10-
pull-requests: write
1113
id-token: write # Required for npm OIDC trusted publishing
1214

1315
concurrency:
@@ -18,6 +20,10 @@ jobs:
1820
prepare:
1921
if: github.repository == 'Fission-AI/OpenSpec' && github.event_name == 'push'
2022
runs-on: ubuntu-latest
23+
permissions:
24+
contents: write
25+
pull-requests: write # changesets opens/updates the Version Packages PR
26+
id-token: write # Required for npm OIDC trusted publishing
2127
steps:
2228
# Generate GitHub App token first - used for checkout and changesets
2329
# This allows git operations to trigger CI workflows on the version PR

.github/workflows/security.yml

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,32 @@ jobs:
8888
if: ${{ !cancelled() }}
8989
continue-on-error: ${{ github.event_name == 'pull_request' }}
9090
run: pnpm audit --audit-level high --dir website
91+
92+
# The website keeps its own lockfile and is never installed or built elsewhere
93+
# in CI, so a website/package.json change — e.g. a security override — that is
94+
# not reflected in website/pnpm-lock.yaml goes unnoticed: the override you think
95+
# patches an advisory may not be in the committed graph at all, and `pnpm audit`
96+
# would happily audit the stale (possibly still-vulnerable) tree. A frozen-lockfile
97+
# install fails fast on that drift. Root drift is already caught by the
98+
# `--frozen-lockfile` installs in ci.yml; this closes the same gap for the website.
99+
# `--ignore-scripts` skips sharp's native build (irrelevant to lockfile validation
100+
# and the usual source of install flake).
101+
website-lockfile:
102+
name: Website Lockfile Drift
103+
runs-on: ubuntu-latest
104+
steps:
105+
- name: Checkout code
106+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
107+
with:
108+
persist-credentials: false
109+
110+
- name: Setup pnpm
111+
uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6
112+
113+
- name: Setup Node.js
114+
uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
115+
with:
116+
node-version: '20.19.0'
117+
118+
- name: Verify website lockfile matches package.json
119+
run: pnpm install --frozen-lockfile --ignore-scripts --dir website

flake.nix

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
inherit (finalAttrs) pname version src;
5252
pnpm = pkgs.pnpm_9;
5353
fetcherVersion = 3;
54-
hash = "sha256-6huf6aAPGkK8Oz6YqbRXObTmFbwRv+6GH5qtNlhlfFw=";
54+
hash = "sha256-w3nzoSXu6eONUDcuzgbGhA0a5ix5zU1QhNIFwwJPnXs=";
5555
};
5656

5757
nativeBuildInputs = with pkgs; [

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@
8686
},
8787
"pnpm": {
8888
"overrides": {
89-
"brace-expansion@<=5.0.7": ">=5.0.8"
89+
"brace-expansion@<=5.0.8": ">=5.0.9 <6",
90+
"postcss@<8.5.23": ">=8.5.23 <9"
9091
}
9192
}
9293
}

pnpm-lock.yaml

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/archive.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,9 @@ async function releaseArchiveClaim(
574574
await claim.handle.close().catch(() => undefined);
575575
if (owned === undefined) return;
576576
try {
577+
// Read between two lstats by design: the identity + content match below
578+
// proves we still own this claim before unlinking it. This is a concurrent-
579+
// change detector, not an fd-less race to "fix" (CodeQL js/file-system-race).
577580
const current = await fs.lstat(claimPath, { bigint: true });
578581
const contents = await fs.readFile(claimPath, 'utf8');
579582
const currentAfterRead = await fs.lstat(claimPath, { bigint: true });
@@ -706,6 +709,9 @@ async function fingerprintPath(filePath: string): Promise<string> {
706709
async function fingerprintMovablePath(filePath: string): Promise<string> {
707710
try {
708711
const entry = await fs.lstat(filePath, { bigint: true });
712+
// Deliberate stat -> read -> re-stat: a concurrent change is DETECTED by the
713+
// statIdentity comparison below and throws. Do not collapse to fd I/O, which
714+
// would pin one inode and blind the detector (CodeQL js/file-system-race).
709715
const hash = createHash('sha256')
710716
.update(await fs.readFile(filePath))
711717
.digest('hex');
@@ -741,6 +747,9 @@ async function fingerprintMovablePath(filePath: string): Promise<string> {
741747
async function fingerprintPortableContent(filePath: string): Promise<string> {
742748
try {
743749
const entry = await fs.lstat(filePath);
750+
// Point-in-time content hash by design (no re-stat): callers compare it
751+
// against a prior fingerprint of the same bytes, so any concurrent change
752+
// surfaces as a hash mismatch (CodeQL js/file-system-race is a false positive here).
744753
const hash = createHash('sha256')
745754
.update(await fs.readFile(filePath))
746755
.digest('hex');
@@ -818,6 +827,9 @@ async function captureSpecSnapshots(mutations: SpecMutation[]): Promise<SpecSnap
818827
let contentExisted = false;
819828
if (outcome === 'write') {
820829
try {
830+
// Best-effort rollback snapshot; a concurrent edit is caught later
831+
// by restoreSpecSnapshots refusing to overwrite non-matching content,
832+
// not here (CodeQL js/file-system-race).
821833
content = await fs.readFile(update.target);
822834
contentExisted = true;
823835
} catch (error) {
@@ -839,6 +851,9 @@ async function captureSpecSnapshots(mutations: SpecMutation[]): Promise<SpecSnap
839851
existed: true,
840852
outcome,
841853
...(outcome === 'write' ? { expectedContent: Buffer.from(rebuilt) } : {}),
854+
// Snapshot read for rollback; restoreSpecSnapshots re-checks this
855+
// content before restoring, so a mid-run change aborts instead of
856+
// clobbering (CodeQL js/file-system-race).
842857
...(stat.isFile() ? { content: await fs.readFile(update.target) } : {}),
843858
...(stat.isFile() ? { mode: stat.mode } : {}),
844859
};
@@ -882,6 +897,9 @@ async function restoreSpecSnapshots(snapshots: SpecSnapshot[]): Promise<void> {
882897
snapshot.symlink !== undefined &&
883898
current.isSymbolicLink() &&
884899
(await fs.readlink(snapshot.target)) === snapshot.symlink;
900+
// Re-read to confirm the target still holds the snapshot content; a
901+
// mismatch means a concurrent edit, and rollback throws below rather
902+
// than overwrite it (CodeQL js/file-system-race is intentional here).
885903
const unchangedFile =
886904
snapshot.symlink === undefined &&
887905
snapshot.content !== undefined &&
@@ -919,6 +937,9 @@ async function restoreSpecSnapshots(snapshots: SpecSnapshot[]): Promise<void> {
919937
`Archive rollback would overwrite a concurrent change at ${snapshot.target}.`
920938
);
921939
}
940+
// Re-read at rollback: only restore when current content matches what
941+
// archive wrote or snapshotted; otherwise abort to preserve a concurrent
942+
// change (CodeQL js/file-system-race is intentional here).
922943
const currentContent = await fs.readFile(snapshot.target);
923944
const originalContent =
924945
snapshot.symlink !== undefined && !snapshot.contentExisted

website/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@
3737
"overrides": {
3838
"postcss": "^8.5.22",
3939
"sharp": "^0.35.3",
40-
"brace-expansion@<=5.0.7": ">=5.0.8"
40+
"brace-expansion@<=5.0.8": ">=5.0.9 <6",
41+
"fast-uri@<3.1.5": "^3.1.5"
4142
}
4243
}
4344
}

website/pnpm-lock.yaml

Lines changed: 10 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)