fix(security): bump @hono/node-server to ^2.0.12 (GHSA-frvp-7c67-39w9) - #379
fix(security): bump @hono/node-server to ^2.0.12 (GHSA-frvp-7c67-39w9)#379seonghobae wants to merge 6 commits into
Conversation
npm audit flagged @hono/node-server <2.0.5 for GHSA-frvp-7c67-39w9 (moderate): a serve-static path traversal on Windows via an encoded backslash (%5C). The fix is only in the 2.x line, so the runtime dependency moves 1.19.14 -> 2.0.12. ScopeWeave calls only the stable `serve({ fetch, port }, cb)` entry and never uses @hono/node-server's serveStatic helper (it serves static assets through its own allowlisted Hono routes), so the vulnerable code path was unreachable; the bump keeps the dependency and both lockfiles clean. Verification: - npm audit --audit-level=moderate -> found 0 vulnerabilities - npm run test:unit -> all pure-math suites pass - npm run test:api -> server boots under v2; API smoke + rate-limit pass - package-lock.json and pnpm-lock.yaml both updated to 2.0.12 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMS Project XML 태그 추출이 동적 정규식에서 Changes보안 대응 및 의존성 정리
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…S finding
The Semgrep multi-language SAST gate flags cloud-sync.js:743 with
javascript.lang.security.audit.detect-non-literal-regexp: parseMsProjectXml's
`tag()` helper built `new RegExp(`<${name}>([^<]*)</${name}>`)` from an
interpolated name. Every caller passes a hardcoded tag ('UID', 'Name',
'OutlineLevel', 'PercentComplete', ...) and the `[^<]*` run is linear, so it
was not actually ReDoS- or injection-exploitable — but the dynamic RegExp trips
the rule and fails the gate for every PR against develop.
Replace it with an indexOf-based extractor that mirrors the original
`/<name>([^<]*)<\/name>/` semantics (the captured run must contain no '<', so a
nested tag yields ''). This removes the flagged pattern entirely at base
instead of suppressing it.
Verification:
- grep 'new RegExp' cloud-sync.js -> none
- npm run test:unit -> MS Project import tests pass (behavior unchanged)
- npm run test:api -> server + rate-limit smoke pass
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head6496fad67b484e7356fd1470888eba0f374b6beb. -
Head SHA:
6496fad67b484e7356fd1470888eba0f374b6beb -
Workflow run: 30522835971
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: msproject.test.mjs"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: msproject.test.mjs"]
R2 --> V2["targeted test run"]
|
… uses npm
Every ScopeWeave PR's OpenCode review is REQUEST_CHANGES because the central
coverage-evidence job fails, which gates approval — blocking the whole PR
train. Root cause is a lockfile-detection asymmetry in the central
ContextualWisdomLab/.github OpenCode dispatch:
- The build-time base materializer (scripts/ci/materialize_base_javascript_packages.py,
fixed in .github#658) already treats a pnpm-lock.yaml with a sibling
package-lock.json and no exact `packageManager: pnpm@X.Y.Z` as a VESTIGIAL
second lockfile and skips pnpm, deferring to the npm path.
- The RUNTIME coverage runner (opencode-review-dispatch.yml select_package_runner)
does NOT mirror that: it selects pnpm the moment a pnpm-lock.yaml exists, then
refuses with "Coverage package runner pnpm requires an exact packageManager
version" and exits 1 ("Coverage sandbox did not publish a regular
authenticated output file; sandbox exit=1").
ScopeWeave is an npm project — package-lock.json is canonical, server-tests.yml
and fuzz.yml use `cache: 'npm'` + `npm ci`, and no workflow / Dockerfile /
compose references pnpm — so the committed pnpm-lock.yaml was a second, unused
lockfile that only pushed the runtime runner into pnpm mode. Removing it lets
the runner use the canonical npm path, so coverage-evidence can run and the
whole develop queue is unblocked (this is the same base-clearing fix as #377).
This is the missing piece of this PR: the @hono/node-server trivy-fs CVE and the
cloud-sync.js ReDoS Semgrep finding were already cleared here; coverage-evidence
was the remaining central-gate blocker.
Verification: `npm run test:unit` passes (all 13 suites); package-lock.json is
untouched and still resolves @hono/node-server 2.0.12.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cloud-sync.js (1)
742-755: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win새로운 실패 경로를 회귀 테스트로 고정해 주세요.
start === -1,end === -1,content.includes('<')분기는 이번 변경으로 명시된 동작입니다. 정상 XML뿐 아니라 누락된 닫는 태그와 중첩 태그 입력에서도 해당 필드가 안전하게 제외되는지 테스트해 두면 이후 파서 변경으로 의미가 다시 바뀌는 것을 방지할 수 있습니다.🤖 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 `@cloud-sync.js` around lines 742 - 755, 지원 함수 tag의 start === -1, end === -1, content.includes('<') 분기에 대한 회귀 테스트를 추가하세요. 정상 XML 추출 동작은 유지하면서 여는 태그 누락, 닫는 태그 누락, 중첩 태그 입력에서는 해당 필드가 제외되거나 빈 값으로 안전하게 처리되는지 검증하세요.
🤖 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.
Nitpick comments:
In `@cloud-sync.js`:
- Around line 742-755: 지원 함수 tag의 start === -1, end === -1,
content.includes('<') 분기에 대한 회귀 테스트를 추가하세요. 정상 XML 추출 동작은 유지하면서 여는 태그 누락, 닫는 태그
누락, 중첩 태그 입력에서는 해당 필드가 제외되거나 빈 값으로 안전하게 처리되는지 검증하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dfedc5dc-a2ef-4e35-848f-04bce190ee96
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
CHANGELOG.mdcloud-sync.jspackage.json
…e is complete The @hono/RegExp/pnpm-lock commits refactored parseMsProjectXml's tag() extractor from a dynamic RegExp to indexOf slicing, adding three defensive branches (open-tag absent → start === -1; close-tag absent → end === -1; nested markup → content contains "<"). The existing happy-path test only exercised the success branch, so the central changed-code coverage gate (javascript_coverage_gate.py fails on incomplete coverage of CHANGED lines) would flag the new branches, and a reviewer requested regression coverage. Adds three targeted cases through the public parseMsProjectXml entry point: - unterminated <Name> (end === -1) → task skipped; - nested markup inside <Name> (content includes "<") → task skipped, mirroring the original /<name>([^<]*)<\/name>/ "no inner <" semantics; - absent optional tags (start === -1) → blank dates and OutlineLevel defaulting to depth 1, task still parsed. Verified: `node tests/unit/msproject.test.mjs` passes; full `npm run test:unit` green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headf6d09d8a56e783a26f7169e2fd1e310abe75a254. -
Head SHA:
f6d09d8a56e783a26f7169e2fd1e310abe75a254 -
Workflow run: 30565119050
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 4
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (5 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (5 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: msproject.test.mjs"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: msproject.test.mjs"]
R2 --> V2["targeted test run"]
…chain) The central coverage-evidence sandbox invokes `npm run coverage`, but the script chained `&& npm run test:fuzz`, which runs `playwright install chromium && playwright test ...`. The coverage sandbox has no Playwright, so the script died with `playwright: not found` (exit 127), so no coverage evidence could be trusted for the head. The coverage-evidence entry point should produce coverage evidence only, not run a browser end-to-end suite: drop the test:fuzz chain so `npm run coverage` writes coverage/coverage-summary.json and exits 0. The CSV formula fuzz E2E still runs via `test:e2e` (Playwright) in the repo's own server-tests workflow; node-native property fuzz still runs via `npm run fuzz` in fuzz.yml — neither is lost. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
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 `@package.json`:
- Line 9: coverage 스크립트와 분리하여 test:fuzz를 독립적인 필수 CI job으로 유지하세요. test:fuzz가
csv_formula_fuzz.spec.js를 직접 실행하도록 구성하고, 정적 커버리지 증거 생성에 의존하지 않게 하세요. CI 설정에서 해당
job이 required 상태로 적용되는지 확인하세요.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93b92d47-9db5-4d5d-b704-d8cb6756e558
📒 Files selected for processing (2)
package.jsontests/unit/msproject.test.mjs
| "scripts": { | ||
| "check:python-docstrings": "node scripts/ci/static_coverage_evidence.mjs docstrings", | ||
| "coverage": "node scripts/ci/static_coverage_evidence.mjs coverage && npm run test:fuzz", | ||
| "coverage": "node scripts/ci/static_coverage_evidence.mjs coverage", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n '"test:fuzz"|npm run test:fuzz' package.json .github scriptsRepository: ContextualWisdomLab/scopeweave
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package.json\n'
cat -n package.json | sed -n '1,120p'
printf '\n## workflow references\n'
rg -n '"test:fuzz"|npm run test:fuzz|test:fuzz' .github package.json scripts -g '!**/node_modules/**'Repository: ContextualWisdomLab/scopeweave
Length of output: 1971
test:fuzz를 별도 필수 CI 단계로 유지하세요. coverage는 이제 정적 커버리지 증거만 생성하므로 tests/e2e/csv_formula_fuzz.spec.js가 함께 실행되지 않습니다. CSV 수식 주입 방어가 빠지지 않도록 이 퍼즈 테스트를 독립적인 required job으로 돌려야 합니다.
🤖 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 `@package.json` at line 9, coverage 스크립트와 분리하여 test:fuzz를 독립적인 필수 CI job으로
유지하세요. test:fuzz가 csv_formula_fuzz.spec.js를 직접 실행하도록 구성하고, 정적 커버리지 증거 생성에 의존하지
않게 하세요. CI 설정에서 해당 job이 required 상태로 적용되는지 확인하세요.
Address CodeRabbit review on #379. Dropping `&& npm run test:fuzz` from the `coverage` script removed the only invocation of tests/e2e/csv_formula_fuzz.spec.js. That path only ran inside the central coverage-evidence sandbox (via `npm run coverage`), where Playwright is unavailable, so the CSV formula injection defense never actually executed in CI. Add it to the cloud-e2e job, which already sets up Playwright and whose config auto-starts the static webServer, so the spec runs against window.csvEscape / window.sanitizeCsvFormulaValue as a real required check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Msz8Dni5zRaqPatNCmZJZ8
|
Superseded by #386 — clean combined fix: @hono/node-server ^2.0.12 (GHSA-frvp-7c67-39w9) + hono ^4.12.32 + server audit CSV whitespace formula guard with regression smoke. Closing to reduce PR queue noise. |
2 participants
Summary
npm auditondevelopflags@hono/node-server<2.0.5for GHSA-frvp-7c67-39w9 (moderate): aserve-staticpath traversal on Windows via an encoded backslash (%5C). The fix exists only in the 2.x line, so this bumps the runtime dependency1.19.14 → 2.0.12(bothpackage-lock.jsonand the trackedpnpm-lock.yamlupdated).Why this is safe
serve({ fetch: app.fetch, port }, cb)entry (server/server.mjs) and never uses@hono/node-server'sserveStatichelper — it serves static assets through its own allowlisted Hono routes inserver/app.mjs. The vulnerable code path was therefore unreachable, but the dependency is bumped so the lockfiles stay clean and the advisory clears.@hono/node-serverv2's runtime requirement.hono+@hono/node-server); this is a version bump of an existing one.Verification
npm audit --audit-level=moderate→ found 0 vulnerabilitiesnpm run test:unit→ all pure-math suites pass (EVM/S-curve, CPM, baselines, workload, …)npm run test:api→ server boots under v2; API smoke (auth·tenancy·RBAC·billing·webhooks) + rate-limit tests passpackage-lock.jsonandpnpm-lock.yamlboth resolve@hono/node-server@2.0.12Files
package.json— dependency version bumppackage-lock.json,pnpm-lock.yaml— regenerated lockfilesCHANGELOG.md—### Securityentry under[Unreleased]Generated by Claude Code
Summary by CodeRabbit
<포함 등)에서도 태스크 처리 결과가 흔들리지 않도록 방어 로직을 보강했습니다.