fix(security): upgrade Hono and harden attachment status refresh - #432
Conversation
server/app.mjs에서 첨부파일(attachments)의 PENDING 상태를 동기화할 때, 기존 for...of 루프 내부에서 jobStatus를 순차적으로 await 하던 로직을 Promise.all(rows.map(...))을 사용하도록 변경했습니다. 이를 통해 첨부파일이 여러 개일 경우 발생하는 네트워크 호출 병목을 효과적으로 줄이고 응답 지연을 방지합니다.
server/app.mjs에서 첨부파일(attachments)의 PENDING 상태를 동기화할 때, 기존 for...of 루프 내부에서 jobStatus를 순차적으로 await 하던 로직을 Promise.all(rows.map(...))을 사용하도록 변경했습니다. 이를 통해 첨부파일이 여러 개일 경우 발생하는 네트워크 호출 병목을 효과적으로 줄이고 응답 지연을 방지합니다. 추가로 CI Trivy 스캔에서 발견된 hono 패키지의 취약점(CVE-2026-69207)을 해결하기 위해 버전을 4.12.32에서 4.13.0으로 업데이트했습니다.
Address CodeRabbit feedback: unbounded Promise.all over all pending attachments could exceed Clearfolio connection/rate limits. Filter to PENDING/RUNNING rows and process in chunks of 5, preserving best-effort stale-status handling. Also revise the .jules/bolt.md guidance to require bounded concurrency for external calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pull request was converted to draft
![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
9746592143d4e87c03b7a95adf678e789d3d352a. - 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:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/30957249613/job/92153149476)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/scopeweave/actions/runs/30957249613/job/92153149476)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (6 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (6 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (7 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (7 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (3 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (3 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (7 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (7 files)"]
R3 --> V3["targeted test run"]
|
|
@opencode-agent review Please independently review the exact current head after all current-head checks complete. Verify the Hono security remediation, bounded attachment-status refresh, per-item and request-wide budgets, Clearfolio response validation and sanitization, internal conversion-identifier redaction, low-cardinality failure metrics, real-world regressions, 100% changed-production coverage/docstrings, MSA boundary, CHANGELOG, and APA 7th doctoring. Submit APPROVE only if no blocker remains; do not bypass repository protections. |
|
Current-head Strix blocker diagnosis for |
|
@opencode-agent review Re-evaluate exact unchanged head |
Dismissed stale automated request: the cited Strix failure was rerun on the exact head and is now terminal-success; current-head Checks are clean.
|
@opencode-agent @cwl-noema-review Formal review-only request for exact current head |
…ation-current-head fix(security): enforce strict revocable sessions on every JWT transport
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: evidence affirmatively supports PR intent (Hono 4.12.32->4.13.0 CVE-2026-69207 remediation in package.json:13 / package-lock.json:387, N+1 removal and bounded refresh in server/attachment_status.mjs, Clearfolio sanitization in server/clearfolio.mjs, token_version revocation in server/auth.mjs, internal jobId redaction in server/app.mjs) with tests and docs updated on the same head; changed-file evidence inspected: server/attachment_status.mjs (CodeGraph current-head source excerpt) and tests/api/session-revocation.test.mjs (focused-hunk receipt line 208). Verification posture: trusted Coverage execution evidence records supported suites (npm run test:unit, npm run test:api, npm run coverage, npm run fuzz) as PASS and Failed GitHub Check evidence records no completed failed checks for this head. Linter/static: repo lint contract is hadolint Dockerfile; no lint findings in current-head evidence. TDD/regression: regression tests added per changed surface — tests/api/session-revocation.test.mjs, tests/api/attachment-status.test.mjs, tests/unit/attachment-status.test.mjs, tests/unit/clearfolio-status-signal.test.mjs, tests/unit/clearfolio-adapter-mock-hmac.test.mjs, tests/unit/coverage-script-contract.test.mjs. Coverage: PASS — supported repository test suites passed per Coverage execution evidence. Docstring coverage: configured repository docstring gates passed or docstring coverage advisory per Coverage execution evidence. DAG: head-flow Mermaid diagram below maps attachment-list request -> refreshAttachmentStatuses (server/attachment_status.mjs) -> clearfolio jobStatus/artifactUrl (server/clearfolio.mjs) -> sanitized fixed-category metrics, and verifyToken/token_version (server/auth.mjs) -> all four JWT transports; reflects base-to-head changed flow (per-row lookup and jobId exposure removed). PoC/execution: no OPENCODE_EXECUTION_RECEIPT browser/e2e tool receipts exist; source-level trace plus committed API/unit tests are cited instead. DDD/domain: refresh is project-scoped; per-tenant data isolation preserved. CDD/context: Clearfolio boundary treats downstream responses as untrusted — status allowlist and scheme/HTTPS-downgrade checks live at the adapter edge. Similar issues: prior bot review nits are resolved on this head; the earlier CHANGES_REQUESTED was dismissed as stale and current-head evidence shows no failed checks and no unresolved threads. Claim/concept check: CHANGELOG.md, docs/deploy.md, and docs/doctoring/* claims match code evidence; Hono remediation claim matches the dependency bump. Standards search: strict HS256/JWT header and token_version revocation follow OWASP session-management guidance documented in docs/doctoring/session-revocation.md; no external standard needed beyond repo doctoring evidence. Compatibility/convention: new names (SCOPEWEAVE_ATTACHMENT_STATUS_, attachmentStatusRefresh metrics, attachment_status.mjs) are multi-word and follow repo snake_case/camelCase conventions; no reserved-word conflicts; internal job_id stays non-exposed. Breaking-change/backcompat: schema unchanged and no migration required (docs/deploy.md rollback section); removal of internal jobId from responses is a documented security change in CHANGELOG.md; legacy sessions lacking the tv claim fail closed by design. Implementation completeness: no placeholder bodies (pass/.../NotImplementedError) found in inspected head source excerpts (attachment_status.mjs constants/normalizers/refresh, app.mjs import wiring and logout-all route). Performance: per-row DB lookup removed; worker pool bounded at 8 (max 32) with per-item abortable timeout and request-wide latency budget. Developer experience: operator controls and canary/rollback guidance documented in docs/deploy.md with metric-interpretation ratios. User experience: stale status preserved on downstream/timeout/malformed/persistence failures so the attachment list stays available; UX surface reviewed is the API list-response contract (non-web backend PR). Visual/DOM: non-web PR — reviewed API/JSON contract, metrics counters, and operator docs instead; no frontend/DOM file in this head's changed files. Accessibility/i18n: no frontend change; changelog keeps Korean entries alongside the new English entries. Supply-chain/license: hono bumped to 4.13.0 in package-lock.json; npm audit --audit-level=high and trivy fs . are the repo security commands; no failed Dependency Review/OSV evidence at head. Packaging: Node ^22.13.0 || >=23.4.0 engines with npm test/coverage/e2e/fuzz contracts present; the unpackaged python file flagged in evidence (tests/config/test_strix_static_repo_adaptations.py) is pre-existing and outside this PR's changed surface. Security/privacy: fail-closed JWT secret startup, strict tv-claim validation, Clearfolio error sanitization, HTTPS-downgrade rejection, and internal-id redaction are all corroborated by committed tests in the PASS coverage run.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including CHANGELOG.md, docs/deploy.md, docs/doctoring/attachment-status-refresh.md, docs/doctoring/session-revocation.md, package-lock.json, and 12 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects CHANGELOG.md to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"server/auth.mjs","line":186,"hypothesis":"logout-all revocation can be bypassed on at least one JWT transport (bearer/calendar/SSE/attachment-view) because verification is not centralized, so a stale token holding the old token_version stays valid after token_version increments.","attack_or_counterexample":"Two devices hold signed tokens for the same user; call POST /api/auth/logout-all on device A, then replay device B's stale token on each of the four transports while presenting the replacement token on bearer.","evidence":"Trusted source trace at server/auth.mjs:186: tests/api/session-revocation.test.mjs (208 current-head lines) asserts stale tokens are rejected on bearer, calendar, SSE, and attachment-view transports and that the replacement token continues through the same boundary; Coverage execution evidence records supported suites as PASS for this head and Failed GitHub Check evidence lists no completed failed checks; source-line-sha256=b771f3c55d11dd3013dd957363d0ffdcd7b74688a68c9c1ea10819dd8a56329e","outcome":"falsified"},{"path":"server/clearfolio.mjs","line":229,"hypothesis":"Downstream Clearfolio error text or network details leak into browser/diagnostic payloads, or whitespace-padded/unknown conversion states are accepted and persisted.","attack_or_counterexample":"Clearfolio returns HTTP 500 with a body containing internal exception text and an artifact link over http://; conversion states 'SUCCEEDED ' (trailing space) and 'weird' are submitted.","evidence":"Trusted source trace at server/clearfolio.mjs:229: tests/unit/clearfolio-status-signal.test.mjs (251 current-head lines) covers rejection of untrusted status states, status-transport leak sanitization, network isolation, and HTTPS-downgrade prevention; the suite is recorded as PASS by Coverage execution evidence for this head; source-line-sha256=b0114d58839ee0c8a505f70e7eeb7ac75ef86f4c755b24f83d138c7e370ecade","outcome":"falsified"},{"path":"server/attachment_status.mjs","line":297,"hypothesis":"The refresh pass performs one database lookup per row (N+1), ignores the request-wide budget, refreshes non-PENDING/RUNNING rows, or mis-reconciles the failed aggregate against the fixed failure-category counters.","attack_or_counterexample":"An attachment list with 200 rows spanning PENDING/RUNNING/SUCCEEDED/FAILED, including rows with blank job_id, refreshed with a 1 ms budget and concurrency 32.","evidence":"Trusted source trace at server/attachment_status.mjs:297: tests/unit/attachment-status.test.mjs (315 current-head lines) covers the bounded engine, deadline enforcement, skipped/deferred separation, and failure-category reconciliation; CodeGraph records refreshAttachmentStatuses (server/attachment_status.mjs:204) with 2 callers in server/app.mjs and tests in tests/unit/attachment-status.test.mjs; Coverage execution evidence records the suites as PASS; source-line-sha256=d10b36aa74a59bcf4a88185837f658afaf3646eff2bb16c3928d0e9335e945d2","outcome":"falsified"},{"path":"tests/api/attachment-status.test.mjs","line":130,"hypothesis":"Internal Clearfolio job identifiers leak through the upload or list API responses.","attack_or_counterexample":"Upload an attachment through the API and list the project attachments, asserting the payload contains no jobId field on either surface.","evidence":"Trusted source trace at tests/api/attachment-status.test.mjs:130: the API test uses behavior-only list assertions and, together with tests/api/smoke.mjs:635-636, asserts identifier redaction; Coverage execution evidence records the API suite as PASS and Changed-files evidence confirms both files are current-head changes; source-line-sha256=29576b54e255e3c948eea5b5904fa38b81682fdd3cbd9ae841ff0e7bd80d394c","outcome":"falsified"}],"residual_risk":"Full hunks of server/app.mjs, server/auth.mjs, and server/clearfolio.mjs were truncated from the inlined evidence (12000-byte excerpt of a 114953-byte diff), so line-level verification of budget arithmetic, AbortSignal forwarding, and claim checks relies on the committed tests plus the trusted Coverage PASS rather than direct hunk reads; ship behind the documented canary and watch the fixed-category counters before raising concurrency or budget."}- Result: APPROVE
- Reason: No active failed checks, no unresolved threads, Coverage execution evidence records supported suites as PASS, and committed tests corroborate the bounded attachment refresh, Clearfolio sanitization, internal-jobId redaction, and cross-transport session revocation claims at head 0055199.
- Head SHA:
0055199096c39874a7d2b5dc086ac2d2750b17ec - Workflow run: 31770259928
- Workflow attempt: 1
2 participants
Summary
This pull request combines the Hono security update with buyer-visible attachment-list and Clearfolio boundary hardening:
honofrom 4.12.32 to 4.13.0, remediating CVE-2026-69207;SELECT job_idlookup;skippeddata-quality cases while reservingdeferredfor valid work not started before the latency budget;jobIdfrom both upload and list JSON responses;test:coverageitself produce exact Istanbul JSON and JSON-summary evidence so central current-head review cannot execute tests without coverage output;PENDING,RUNNING,SUCCEEDED, andFAILEDconversion states; reject whitespace-padded or unknown states, malformed links, non-HTTP(S) schemes, and HTTPS-to-HTTP downgrade links;docs/doctoring/; andThe refresh scheduler remains isolated in
server/attachment_status.mjs, independent of Hono and SQLite, so it can be reused by a future standalone service adapter while the current product remains independently operable.Regression and quality evidence
Focused coverage proves:
jobId;jsonplusjson-summaryreporters; andserver/attachment_status.mjsmust retain 100% statement, branch, function, and line coverage, with current-head coverage and docstring gates required for every changed production path.Verification
Predecessor exact-tree run
30902156688completed successfully at commit140bf95919ff77404d8ed4338667e32b4b973ea9, executing full unit/API/coverage, configured docstring evidence, nine cloud Playwright E2E tests, explicit 100% coverage checks forserver/attachment_status.mjs, andgit diff --check.The exact current head additionally separates skipped/deferred semantics, adds low-cardinality failure-category metrics, strengthens the coverage reporter contract, simplifies validated Clearfolio submission parsing, removes the internal conversion identifier from the upload response, and updates focused API tests plus deployment, CHANGELOG, and doctoring evidence. Every repository and central required check must therefore succeed again on the exact current head, followed by independent current-head approval, before merge.
Standards traceability
docs/doctoring/attachment-status-refresh.mdrecords the MSA boundary, verification contract, operational acceptance criteria, and APA 7th references to OWASP API4:2023 Unrestricted Resource Consumption and API10:2023 Unsafe Consumption of APIs. The implementation follows those risk-reduction controls without claiming formal OWASP certification.Release note
CHANGELOG.mdrecords bounded refresh, request budgeting, malformed-response isolation, exact conversion-state validation, partial-failure behavior, skipped/deferred separation, sanitized category metrics, upload/list identifier hiding, downstream error sanitization, and artifact-link downgrade prevention under Unreleased.Closes #408. Supersedes #419, #420, and #431.
Summary by CodeRabbit
개선 사항
보안
문서 및 테스트