security(auth): equalize login verification work for unknown users - #692
security(auth): equalize login verification work for unknown users#692seonghobae wants to merge 6 commits into
Conversation
- Adds a globally initialized `DUMMY_PASSWORD_HASH` - Refactors `/api/auth/login` to unconditionally verify the dummy hash when a user isn't found - Coerces candidate passwords to string avoiding TypeErrors on objects - Prevents attacker from enumerating valid users via timing difference
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
- Stabilizes auth response execution behavior
- Adds a globally initialized DUMMY_PASSWORD_HASH - Refactors /api/auth/login to unconditionally verify the dummy hash when a user isn't found - Coerces candidate passwords to string avoiding TypeErrors on objects - Prevents attacker from enumerating valid users via timing difference
![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 could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
f8c75d0c8e457d8fe77091e2ab5b210aaad51e73. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- CodeQL PR/CodeQL compatibility analysis (actions): FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746618)
- CodeQL PR/CodeQL compatibility analysis (javascript-typescript): FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746621)
- CodeQL PR/CodeQL compatibility analysis (python): FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746644)
- CodeQL compatibility analysis (actions) check run: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746618)
- CodeQL compatibility analysis (javascript-typescript) check run: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746621)
- CodeQL compatibility analysis (python) check run: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291726/job/103130746644)
- Required Noema Review/noema-review: FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556290511/job/103130600180)
- Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291695/job/103130565799)
- noema-review check run: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556290511/job/103130600180)
- trivy-fs check run: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/34556291695/job/103130565799)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: sentinel.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: sentinel.md"]
R1 --> V1["required checks"]
Evidence --> S2["TypeScript/JavaScript: app.mjs"]
S2 --> I2["TypeScript or JavaScript runtime"]
I2 --> R2["Review risk: TypeScript/JavaScript: app.mjs"]
R2 --> V2["package test plus coverage"]
OpenCode Review Overview
|
seonghobae
changed the title
seonghobae
changed the title
1 participant
로그인에서 존재하지 않는 계정만 scrypt 검증을 건너뛰던 큰 비용 차이를 제거하는 방향은 유효합니다. 다만 이를 곧바로 ‘timing-safe’ 또는 특정 severity의 완전한 취약점 해소로 과장하지 않습니다.
현재 exact head:
a12d8c3718617d53664b535c495ed995fe38eeeaprotected base:
develop@2c328875e00e86537df3e965170be80532571cad상태: Draft
현재 protected-base 대비 유효 delta는 세 경로뿐입니다.
server/app.mjs: 모듈 로드시 실제hashPassword()와 같은 scrypt 표현의 dummy hash를 만들고, 로그인마다 실제 hash 또는 dummy hash를 대상으로verifyPassword()를 한 번 호출합니다. 계정 부재·비문자 password·불일치 모두 기존401 { error: 'invalid credentials' }경계를 유지합니다.tests/api/login-enumeration-contract.test.mjs: 존재하는 사용자+오답과 존재하지 않는 사용자+동일 형식 password가 같은 401 status/body를 노출하는 API contract를 실제 Hono request 경계에서 검증합니다.package.json: 위 regression을 기존test:api/coverage 실행 경로에 포함합니다.원 branch의
.jules/sentinel.md에는 ‘timing attack이 해결됐다’는 저장소-wide generated doctrine이 추가돼 있었지만, 현실 timing 분포나 deterministic KDF invocation evidence 없이 일반화하기에는 이릅니다. protected Sentinel blob을 ordinary-forward로 복원해 doctrine delta는 0으로 만들었습니다.남은 RED/GREEN acceptance는 다음과 같습니다.
verifyPassword()invocation count 자체를 관찰하지 않습니다. production auth boundary를 과도하게 test-only seam으로 오염시키지 않는 범위에서, 실제/unknown 두 경로가 동일 KDF cost class를 한 번 수행한다는 deterministic evidence가 추가로 필요합니다.hashPassword()경로와 같은 scrypt representation/cost parameter를 계속 사용한다는 invariant를 유지합니다. 별도 저비용 dummy primitive로 바꾸면 안 됩니다..githubowner path에서만 수리합니다. 이 leaf에서 no-op retrigger, synthetic status 또는 gate 완화로 숨기지 않습니다.현재 변경은 account-existence에 따른 가장 큰 ‘KDF 실행 vs 미실행’ 구조 차이를 줄이고 API failure surface를 동일하게 고정합니다. 네트워크 환경에서 관측 가능한 완전한 constant-time authentication을 주장하지 않습니다.