fix(security): JWT secret fail-closed + MS Project XML ReDoS removal - #387
Closed
seonghobae wants to merge 1 commit into
Closed
fix(security): JWT secret fail-closed + MS Project XML ReDoS removal#387seonghobae wants to merge 1 commit into
seonghobae wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthrough이 PR은 SCOPEWEAVE_JWT_SECRET 검증을 경고 방식에서 시작 실패 방식으로 강화하고, docker-compose.yml의 불안전한 기본값을 제거하며, 관련 문서와 테스트를 업데이트한다. 별개로 cloud-sync.js의 XML 태그 추출 함수를 정규식 기반에서 문자열 인덱스 기반으로 교체하여 ReDoS 위험을 제거한다. ChangesJWT 시크릿 필수화
XML 태그 추출 안전성 개선
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Test as auth-secret.test.mjs
participant ChildProcess
participant AuthModule as auth.mjs
Test->>ChildProcess: SCOPEWEAVE_JWT_SECRET 설정 후 실행
ChildProcess->>AuthModule: 모듈 로드
AuthModule->>AuthModule: 타입/길이/리터럴 검증
alt 검증 실패
AuthModule-->>ChildProcess: Error 발생, 종료 코드 비0
else 검증 성공
AuthModule-->>ChildProcess: 정상 시작, 종료 코드 0
end
ChildProcess-->>Test: 종료 코드 및 stderr 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
- server/auth.mjs: refuse startup when SCOPEWEAVE_JWT_SECRET is missing, <32 non-whitespace chars, or an unexpanded compose literal (no more hardcoded dev-insecure-secret fallback) - docker-compose.yml / Dockerfile.server: drop insecure default secret - cloud-sync parseMsProjectXml: replace dynamic RegExp tag parse with indexOf/slice (eliminates ReDoS / Semgrep non-literal-regexp surface) - Add tests/api/auth-secret.test.mjs; wire into test:api; strengthen MS Project unit case for regex-metachar task names - Docs (README, deploy, CLAUDE): required secret contract Supersedes open PR #329 with a clean develop rebase of the same contract.
This was referenced Jul 31, 2026
seonghobae
force-pushed
the
fix/jwt-fail-closed-xml-redos
branch
from
July 31, 2026 13:07
8bdad23 to
3728907
Compare
seonghobae
changed the base branch from
develop
to
fix/security-hono-csv-hardening
July 31, 2026 13:07
Contributor
Author
This was referenced Jul 31, 2026
![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 `@tests/unit/msproject.test.mjs`:
- Around line 58-62: 보강된 테스트가 동적 정규식 제거를 실제로 검증하지 못하고 있습니다.
tests/unit/msproject.test.mjs의 parseMsProjectXml 테스트에서 정규식 메타문자를 XML 본문이 아닌 tag의
name 인자로 전달하는 경계를 직접 검증하도록 수정하고, 작업 이름 보존 검증과 ReDoS 회귀 검증을 분리하세요.
🪄 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: d74a480f-d313-4d19-8be3-56dfa64ded70
📒 Files selected for processing (13)
CLAUDE.mdDockerfile.serverREADME.mdcloud-sync.jsdocker-compose.ymldocs/deploy.mdpackage.jsonserver/auth.mjstests/api/auth-secret.test.mjstests/api/ratelimit.test.mjstests/api/smoke.mjstests/e2e/cloud.spec.jstests/unit/msproject.test.mjs
Comment on lines
+58
to
+62
| const literalRegexText = parseMsProjectXml( | ||
| '<Project><Tasks><Task><UID>6</UID><Name>Regex [.*+?] text</Name><OutlineLevel>1</OutlineLevel></Task></Tasks></Project>', | ||
| ); | ||
| assert.equal(literalRegexText[0].phase, 'Regex [.*+?] text', 'tag extraction treats task content as literal text'); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
ReDoS 회귀 테스트가 실제 변경을 검증하도록 보강하세요.
현재 [.*+?]는 XML <Name> 본문에 있습니다. 이 문자열은 tag의 name 인자에 전달되지 않습니다. 따라서 기존 new RegExp(...) 구현도 이 테스트를 통과할 수 있습니다. 현재 테스트는 작업 이름 보존만 검증하며 동적 정규식 제거를 검증하지 않습니다. name이 외부 입력이 될 수 있다면 해당 경계를 직접 테스트하세요. 그렇지 않다면 테스트 목적을 작업 이름 보존으로 명확히 하고 ReDoS 검증은 별도로 추가하세요.
제공된 변경 설명의 기존 new RegExp(...) 동작과 현재 테스트 입력을 비교한 판단입니다.
🤖 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 `@tests/unit/msproject.test.mjs` around lines 58 - 62, 보강된 테스트가 동적 정규식 제거를 실제로
검증하지 못하고 있습니다. tests/unit/msproject.test.mjs의 parseMsProjectXml 테스트에서 정규식 메타문자를
XML 본문이 아닌 tag의 name 인자로 전달하는 경계를 직접 검증하도록 수정하고, 작업 이름 보존 검증과 ReDoS 회귀 검증을
분리하세요.
seonghobae
marked this pull request as draft
July 31, 2026 13:10
Contributor
Author
This was referenced Jul 31, 2026
Contributor
Author
|
Superseded: JWT fail-closed + MS Project XML ReDoS folded into #386 (single security PR). Close as duplicate of the consolidated train. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Production auth must not mint JWTs with a hardcoded or weak secret.
server/auth.mjs): startup throws unlessSCOPEWEAVE_JWT_SECRETis a string with ≥32 non-whitespace characters and is not an unexpanded Compose literal (\${SCOPEWEAVE_JWT_SECRET…}).insecure-dev-secret-CHANGE-MEdefault; document required secret.parseMsProjectXmluses linearindexOf/sliceinstead ofnew RegExp(name).tests/api/auth-secret.test.mjs(wired intotest:api); smoke/ratelimit/cloud e2e use a 32-char test secret; unit case for regex-metachar task names.docs/deploy.md/ CLAUDE.md match the shipped contract.Supersedes
Closes the same gap as open #329 with a clean reimplementation on current
develop(no stale base). #329 can be closed after this opens.Related
@hono/node-server2.0.12 + audit CSV whitespace formula guard (still needed for GHSA + trivy base).Verification
npm run test:unit✅npm run test:api✅ (auth-secret + smoke + ratelimit)Summary by CodeRabbit
보안 개선
문서
테스트