chore(security): add security policy, dependabot config, and config key guards#1415
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConfiguration key paths now reject prototype-chain segments during validation and nested mutations, including with ChangesConfiguration path hardening
Linear heading parsing
Security operations and supply-chain controls
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/core/config-schema.test.ts`:
- Around line 379-385: Add an assertion in the setNestedValue test for each
unsafe path that the target obj remains empty after the write attempt, while
retaining the existing Object.prototype pollution checks. Update only the test
named “setNestedValue leaves the prototype untouched for…” and verify it with
the specified Vitest command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dfca805b-060a-474d-9363-b6f939a8ba2b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/harden-config-key-paths.md.github/dependabot.ymlSECURITY.mdpackage.jsonsrc/commands/config.tssrc/core/config-schema.tstest/core/config-schema.test.ts
| it.each(unsafePaths)('setNestedValue leaves the prototype untouched for "%s"', (path) => { | ||
| const obj: Record<string, unknown> = {}; | ||
| setNestedValue(obj, path, 'polluted'); | ||
|
|
||
| expect(({} as Record<string, unknown>).polluted).toBeUndefined(); | ||
| expect(Object.prototype).not.toHaveProperty('polluted'); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Assert that unsafe writes leave the target object unchanged.
The prototype assertions alone would still pass for some guard regressions that create own constructor/prototype properties. Assert obj remains empty too.
Proposed test strengthening
setNestedValue(obj, path, 'polluted');
+ expect(obj).toEqual({});
expect(({} as Record<string, unknown>).polluted).toBeUndefined();Run pnpm exec vitest run test/core/config-schema.test.ts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it.each(unsafePaths)('setNestedValue leaves the prototype untouched for "%s"', (path) => { | |
| const obj: Record<string, unknown> = {}; | |
| setNestedValue(obj, path, 'polluted'); | |
| expect(({} as Record<string, unknown>).polluted).toBeUndefined(); | |
| expect(Object.prototype).not.toHaveProperty('polluted'); | |
| }); | |
| it.each(unsafePaths)('setNestedValue leaves the prototype untouched for "%s"', (path) => { | |
| const obj: Record<string, unknown> = {}; | |
| setNestedValue(obj, path, 'polluted'); | |
| expect(obj).toEqual({}); | |
| expect(({} as Record<string, unknown>).polluted).toBeUndefined(); | |
| expect(Object.prototype).not.toHaveProperty('polluted'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/core/config-schema.test.ts` around lines 379 - 385, Add an assertion in
the setNestedValue test for each unsafe path that the target obj remains empty
after the write attempt, while retaining the existing Object.prototype pollution
checks. Update only the test named “setNestedValue leaves the prototype
untouched for…” and verify it with the specified Vitest command.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flake.nix`:
- Around line 53-54: Replace the empty hash in the fetchPnpmDeps configuration
with the valid pinned sha256 hash generated by running scripts/update-flake.sh.
Preserve fetcherVersion and commit the resulting sha256-... value so Nix build
and update validation can resolve dependencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
5d673cf to
26600a8
Compare
…ey guards Adds a SECURITY.md with a private disclosure path and an explicit threat model, a Dependabot configuration covering the CLI package, the docs site, and CI actions, and closes a prototype-pollution path in `config set`. `--allow-unknown` was meant to relax the known-key check but skipped every key check, so `openspec config set --allow-unknown __proto__.polluted x` reported success and assigned onto Object.prototype for the process lifetime. Unsafe segments are now rejected at the command layer regardless of `--allow-unknown`, and setNestedValue/deleteNestedValue refuse them for any caller. Also bumps the bundled yaml dependency from 2.8.2 to 2.9.0, the only advisory in this repo that affects code shipped in the npm package. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fetchPnpmDeps pins a fixed-output hash over the whole dependency set, so changing pnpm-lock.yaml invalidates it. Recovered the new value from a hash-mismatch build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refreshes both lockfiles so every open advisory in the CLI package is resolved, replaces a quadratically-backtracking heading parser, and adds the automation to catch the next one. Dependency refresh (in-range, lockfile only): brace-expansion, flatted, js-yaml, minimatch, postcss, rollup, and vite all move to patched versions in the root lockfile; fast-uri and brace-expansion move in the website lockfile. Only @changesets/cli needed a declared floor bump, to reach a patched js-yaml. Production dependencies now report zero advisories. extractFirstPurposeLine parsed ATX headings with /\s+#+\s*$/, which backtracks quadratically on a whitespace-padded title. Replaced with a linear hand-rolled scan, verified identical to the old implementation across 303,000 generated inputs. Automation: a Security workflow runs dependency review on pull requests, blocks on advisories in published dependencies, and re-audits weekly; every GitHub Action is pinned to a commit SHA so a moved tag cannot change what CI executes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
26600a8 to
89240c1
Compare
Deploying openspec-docs with
|
| Latest commit: |
89240c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cc077a6b.openspec-docs.pages.dev |
| Branch Preview URL: | https://chore-security-hardening.openspec-docs.pages.dev |
Nothing is installed there, so setup-node's cache-save post step failed on the missing store path even though both audit steps passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-prepare.yml:
- Around line 27-30: Update the create-github-app-token step in the release
workflow to add explicit permission-* inputs limited to the permissions required
by checkout and Changesets, rather than inheriting the full installation grant.
Keep the existing app-id and private-key configuration unchanged and avoid
granting unrelated permissions.
- Around line 32-35: Disable credential persistence in the prepare checkout step
by setting persist-credentials to false in .github/workflows/release-prepare.yml
lines 32-35, then provide the GitHub App token explicitly to the
changesets/action step at lines 73-75 so only that step can push.
In @.github/workflows/security.yml:
- Around line 17-18: Update the dependency-review job in the security workflow
to grant pull-requests: write alongside the existing contents: read permission,
preserving comment-summary-in-pr: on-failure so failure summaries can be posted.
In `@src/core/references.ts`:
- Around line 79-125: Update parseHeadingTitle to accept up to three leading
spaces, recognize an opening hash run ending at the line boundary as a valid
empty heading, and reject four-or-more-space indentation. Replace the \s
delimiter checks used by parseHeadingTitle and stripClosingSequence with
spaces/tabs-only matching, while preserving closing-sequence behavior; add
regressions for empty, indented, and four-space-rejected ATX headings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2cfbb5d8-944d-4974-9e89-afa1cb6fec39
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlwebsite/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.changeset/harden-config-key-paths.md.changeset/linear-heading-parse.md.github/dependabot.yml.github/workflows/ci.yml.github/workflows/release-prepare.yml.github/workflows/security.ymlSECURITY.mdflake.nixpackage.jsonsrc/commands/config.tssrc/core/config-schema.tssrc/core/references.tstest/core/config-schema.test.tstest/core/references.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/dependabot.yml
- test/core/config-schema.test.ts
- flake.nix
- src/core/config-schema.ts
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 | ||
| with: | ||
| app-id: ${{ vars.APP_ID }} | ||
| private-key: ${{ secrets.APP_PRIVATE_KEY }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## release-prepare workflow\n'
cat -n .github/workflows/release-prepare.yml | sed -n '1,220p'
printf '\n## workflow references to create-github-app-token / checkout\n'
rg -n "create-github-app-token|actions/checkout|persist-credentials|permission-" .github/workflows -SRepository: Fission-AI/OpenSpec
Length of output: 9997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## action docs / README references in repo\n'
rg -n "create-github-app-token|persist-credentials|permission-.*inputs|checkout" -S .github README* docs* . 2>/dev/null | sed -n '1,220p'Repository: Fission-AI/OpenSpec
Length of output: 31116
🌐 Web query:
actions/create-github-app-token permissions no permission-* inputs grants all available permissions
💡 Result:
The actions/create-github-app-token GitHub Action does not have a single input that automatically grants all available permissions to the generated token [1][2]. Instead, it uses a granular system where you can specify permissions using individual inputs prefixed with permission- (e.g., permission-issues: write, permission-pull-requests: read) [1][3]. By default, if you do not provide any permission-* inputs, the generated token automatically inherits all permissions currently granted to the GitHub App installation [1][3][4]. This default behavior effectively grants all permissions the app possesses, which is why the action recommends explicitly listing only the permissions required for your specific use case to follow the principle of least privilege [1][4]. If you were to manually list every available permission to mimic granting all permissions, you would need to define each required permission as a separate input in your workflow YAML [1][3]. Please note that requesting a permission in your workflow that the GitHub App installation itself does not have will result in an error [1][4][5].
Citations:
- 1: https://github.com/actions/create-github-app-token
- 2: https://deepwiki.com/actions/create-github-app-token/4.1-action-inputs
- 3: https://deepwiki.com/actions/create-github-app-token/4.2-permission-system
- 4: https://github.com/actions/create-github-app-token/blob/5c1e00d1af39c515564da2ae21cc9dedf0793346/README.md
- 5: https://github.com/marketplace/actions/create-github-app-token
Limit the GitHub App token to the permissions this release needs.
Without any permission-* inputs, actions/create-github-app-token inherits every permission granted to the app installation. Narrow it to the small set this job actually uses, so checkout and Changesets don't get the full installation grant.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 27-27: dangerous use of GitHub App tokens (github-app): app token inherits blanket installation permissions
(github-app)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-prepare.yml around lines 27 - 30, Update the
create-github-app-token step in the release workflow to add explicit
permission-* inputs limited to the permissions required by checkout and
Changesets, rather than inheriting the full installation grant. Keep the
existing app-id and private-key configuration unchanged and avoid granting
unrelated permissions.
Source: Linters/SAST tools
| - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 | ||
| with: | ||
| fetch-depth: 0 | ||
| token: ${{ steps.app-token.outputs.token }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release-prepare.yml"
echo "== outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== relevant lines =="
nl -ba "$FILE" | sed -n '1,220p'
echo
echo "== checkout and git push references =="
rg -n "actions/checkout|git push|persist-credentials|token:|github.token|app-token|changeset|release" "$FILE"Repository: Fission-AI/OpenSpec
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release-prepare.yml"
echo "== file metadata =="
wc -l "$FILE"
echo
echo "== lines 1-140 =="
sed -n '1,140p' "$FILE" | nl -baRepository: Fission-AI/OpenSpec
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release-prepare.yml"
wc -l "$FILE"
sed -n '1,180p' "$FILE" | nl -ba
rg -n "actions/checkout|git push|persist-credentials|token:|github.token|app-token|changeset|release" "$FILE"Repository: Fission-AI/OpenSpec
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/release-prepare.yml"
echo "== top of file (1-120) =="
sed -n '1,120p' "$FILE" | awk '{printf "%4d %s\n", NR, $0}'
echo
echo "== bottom of file (121-220) =="
sed -n '121,220p' "$FILE" | awk '{printf "%4d %s\n", NR+120, $0}'
echo
echo "== credential and push references =="
rg -n "actions/checkout|persist-credentials|git push|git config|token:|github.token|app-token|changeset|release" "$FILE"Repository: Fission-AI/OpenSpec
Length of output: 10396
Disable credential persistence on the prepare checkout (.github/workflows/release-prepare.yml#L32-L35). The GitHub App token is only needed by changesets/action; leaving it in local git config exposes it to the later install/release steps. Set persist-credentials: false here and pass the token only to the step that pushes.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 32-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 1 file
.github/workflows/release-prepare.yml#L32-L35(this comment).github/workflows/release-prepare.yml#L73-L75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-prepare.yml around lines 32 - 35, Disable
credential persistence in the prepare checkout step by setting
persist-credentials to false in .github/workflows/release-prepare.yml lines
32-35, then provide the GitHub App token explicitly to the changesets/action
step at lines 73-75 so only that step can push.
Source: Linters/SAST tools
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file =="
git ls-files .github/workflows/security.yml
echo
cat -n .github/workflows/security.yml | sed -n '1,220p'
echo
echo "== search for dependency-review-action usage =="
rg -n "dependency-review-action|comment-summary-in-pr|pull-requests: write|permissions:" .github/workflows -S
echo
echo "== candidate docs references in repo =="
rg -n "dependency-review-action|comment-summary-in-pr" -S .Repository: Fission-AI/OpenSpec
Length of output: 3310
🌐 Web query:
actions/dependency-review-action comment-summary-in-pr pull-requests write permissions
💡 Result:
To use the comment-summary-in-pr feature in the actions/dependency-review-action, you must explicitly grant the pull-requests: write permission to your GitHub Actions workflow or job [1][2]. By default, the action does not post comments, and the comment-summary-in-pr setting is disabled (set to never) [1][3]. When enabled (by setting it to always or on-failure), the action requires write access to pull requests to create or update the summary comment [1][2]. With each execution, the action will overwrite the existing comment in the pull request [1][4]. You can configure these permissions in your workflow YAML file as follows [2][5]: permissions: contents: read pull-requests: write jobs: dependency-review: runs-on: ubuntu-latest steps: - name: Dependency Review uses: actions/dependency-review-action@v4 with: comment-summary-in-pr: always
Citations:
- 1: https://github.com/actions/dependency-review-action
- 2: https://github.com/actions/dependency-review-action/blob/main/docs/examples.md
- 3: https://github.com/marketplace/actions/dependency-review
- 4: https://github.com/actions/dependency-review-action/blob/67d4f4bd7a9b17a0db54d2a7519187c65e339de8/README.md
- 5: https://github.com/actions/dependency-review-action/blob/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/security.yml | sed -n '1,220p'
rg -n "dependency-review-action|comment-summary-in-pr|pull-requests: write|permissions:" .github/workflows -SRepository: Fission-AI/OpenSpec
Length of output: 3168
Grant pull-requests: write to dependency-review
comment-summary-in-pr: on-failure needs pull-requests: write; with only contents: read, the summary comment can't be posted. Add that permission on this job, or disable the summary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security.yml around lines 17 - 18, Update the
dependency-review job in the security workflow to grant pull-requests: write
alongside the existing contents: read permission, preserving
comment-summary-in-pr: on-failure so failure summaries can be posted.
| const WHITESPACE = /\s/; | ||
|
|
||
| /** | ||
| * Drop a CommonMark closing sequence (`## Purpose ##`). The closing run only | ||
| * counts when whitespace precedes it, so `Purpose###` keeps its hashes. | ||
| * Scans from the end so the cost stays linear in the title length. | ||
| */ | ||
| function stripClosingSequence(title: string): string { | ||
| let end = title.length; | ||
| while (end > 0 && WHITESPACE.test(title[end - 1])) { | ||
| end--; | ||
| } | ||
|
|
||
| const hashEnd = end; | ||
| while (end > 0 && title[end - 1] === '#') { | ||
| end--; | ||
| } | ||
|
|
||
| const noClosingRun = end === hashEnd; | ||
| const missingLeadingSpace = end === 0 || !WHITESPACE.test(title[end - 1]); | ||
| if (noClosingRun || missingLeadingSpace) { | ||
| return title.trim(); | ||
| } | ||
|
|
||
| return title.slice(0, end).trim(); | ||
| } | ||
|
|
||
| /** | ||
| * Heading title, or null when the line is not an ATX heading. Hand-rolled | ||
| * rather than a regex so a title padded with whitespace cannot backtrack. | ||
| */ | ||
| function parseHeadingTitle(line: string): string | null { | ||
| let level = 0; | ||
| while (level < 6 && level < line.length && line[level] === '#') { | ||
| level++; | ||
| } | ||
| if (level === 0 || level >= line.length || !WHITESPACE.test(line[level])) { | ||
| return null; | ||
| } | ||
|
|
||
| let start = level; | ||
| while (start < line.length && WHITESPACE.test(line[start])) { | ||
| start++; | ||
| } | ||
|
|
||
| return stripClosingSequence(line.slice(start)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Recognize empty and indented ATX headings.
## returns null, and ## Purpose is rejected, although both are valid CommonMark ATX headings. An empty heading inside Purpose therefore fails to terminate the section and can leak following text as the summary. Support end-of-line after the opening run, up to three leading spaces, and use only spaces/tabs—not \s—for ATX delimiters; add regressions including four-space rejection. (spec.commonmark.org)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/references.ts` around lines 79 - 125, Update parseHeadingTitle to
accept up to three leading spaces, recognize an opening hash run ending at the
line boundary as a valid empty heading, and reject four-or-more-space
indentation. Replace the \s delimiter checks used by parseHeadingTitle and
stripClosingSequence with spaces/tabs-only matching, while preserving
closing-sequence behavior; add regressions for empty, indented, and
four-space-rejected ATX headings.
…permission gap Ran opengrep against the repository to check the claims in #1414. Of 208 findings, 201 are one path-traversal rule firing on joins built from module constants, argv, or readdir entry names; 1 non-literal-regexp is fed only by LEGACY_SLASH_COMMAND_PATHS. Neither is reachable from untrusted input. The actionable results are applied here. - dependabot: add a cooldown so a freshly published version is not adopted immediately. Security updates ignore the cooldown, so this delays only routine bumps, long enough for a compromised release to be yanked. - getNestedValue now refuses prototype-reaching segments, matching the guards already on setNestedValue and deleteNestedValue. - dependency-review no longer asks to comment on the pull request. That needs `pull-requests: write`, which the workflow does not grant and a fork's token never gets, so a real finding would have failed on the comment instead of reporting the vulnerable dependency. - SECURITY.md: show the command that proves build tooling is absent from an installed copy, rather than asking readers to take it on trust. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dependabot rejected the whole config file, which would have silently disabled every version update. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The published-dependency audit no longer fails the job. A newly published advisory should not block an unrelated pull request, and the step depends on registry availability; Dependabot alerts and dependency review remain the gates. SECURITY.md is corrected to match — it claimed the audit was blocking. Also documents what the CLI does on your machine, all verified rather than asserted: the install script prints one line and makes no network request or file write; every shell-invoking call uses a fixed literal while anything carrying user input uses an argument array with shell:false; telemetry sends a command name, a version, and a local random UUID, with IP capture disabled. Secret scanning is now listed, confirmed enabled by a repository admin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The runtime hardening looks sound, but the final commit makes pnpm audit --prod non-blocking on every event. Please keep it advisory on pull requests if desired while making scheduled and main/push production audits blocking, otherwise a newly published high-severity runtime advisory cannot fail any security run unless a dependency changes.
Making the step advisory on every event meant a newly published high-severity advisory in a shipped dependency could not fail any run unless a dependency changed. It stays advisory on pull requests, so an unrelated change is never blocked by an advisory published that morning, and blocks on the weekly schedule and on pushes to main, where a failure is the signal rather than a tax on someone else's work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Approved at exact head af523218. The production audit is now advisory only on pull requests and blocking on scheduled, push, and manual runs; the documentation matches, pnpm audit --prod is clean, and the full exact-head CI/security matrix passes.
…ee them (Fission-AI#1425) * fix(config): guard prototype keys at the write, not behind a helper The guards added in Fission-AI#1415 are effective — Object.prototype is never touched — but they sit at the top of the function and delegate to a Set lookup, which CodeQL cannot follow. Three prototype-pollution alerts stayed open on src/core/config-schema.ts after that PR merged, on exactly the code it hardened. Each key segment is now compared literally in the loop that performs the write. Same behavior for every input, including the empty path and nested creation; the check is simply local to the danger and visible to a reader and to the analyzer. Also tightens a feedback test that matched the issue URL with a substring, which CodeQL rated high. It now parses the URL and compares origin and pathname, so a lookalike host cannot satisfy the assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(config): validate the whole key path before writing The first attempt at making the guard analyzer-visible moved the check into the write loop, which changed behavior: a path whose unsafe segment came after a safe one created the intermediate objects for the safe prefix before bailing out. `setNestedValue({a:'x'}, 'b.constructor.c', v)` left behind `b: {}` where the previous implementation wrote nothing. A differential run against the implementation on main caught it — 47,782 mismatches in 400,000 cases. The literal comparisons stay, but they now scan the whole path before any mutation, so a rejected key leaves the object untouched. Re-run of the same comparison: 0 mismatches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(config): pin the partial-write behavior of a rejected key path The fix landed without a test that would fail if the guard moved back into the write loop, so the regression could return unnoticed. The existing prototype-pollution tests only assert Object.prototype, and every path they use starts with an unsafe segment on an empty object, so no intermediate object is created before the guard trips. Adds cases that put the unsafe segment after a safe one and assert the whole target, plus one for a trailing unsafe segment, where the debris is a re-parented prototype rather than an extra key and a structural comparison alone would miss it. Verified by reintroducing the regression: 5 of the new assertions fail against the buggy build and pass against the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Status: Approved, all checks green.
Answers #1414, where a user's security team rejected OpenSpec after running two scanners. Everything real is fixed, and the rest is documented so the next scan takes minutes to triage instead of a thread.
What changed
Dependencies. Every advisory affecting the published package is gone. One build-only advisory remains (
ajv, pinned upstream by eslint). All of it was a stale lockfile — the fixes landed inside existing version ranges, so no declared dependency moved except@changesets/cli.Two real bugs, neither found by their scanners.
openspec config set --allow-unknown __proto__.polluted xreported success and wrote ontoObject.prototype. The flag was meant to relax the unknown-key check; it was skipping every key check.Automation, so this does not recur. A
SECURITY.mdwith a private disclosure path and an explicit threat model, Dependabot updates with a release cooldown, dependency review on every pull request, a scheduled audit, and all nine GitHub Actions pinned to commit SHAs.Proof
The dependency claim is checkable in two commands:
We also ran the reporter's own scanner: 208 findings, and 201 of them are a single rule (
path-join-resolve-traversal) firing on paths built from module constants, command-line arguments, or filenames fromreaddir. None takes untrusted input.openspec initNotes
sharpstays flagged in the docs site.nextpins it exactly, and the site is a static export that never importsnext/image, so the optimizer that uses it never runs.flake.nixpins a hash over the whole dependency set, so any future lockfile change needs it repinned.security-extendedquery suite.Refs #1414
🤖 Generated with Claude Code