fix(security): upgrade Hono and harden attachment status refresh - #419
dependabot[bot] wants to merge 36 commits into
Conversation
dependabot
Bot
added
dependencies
dependabot
Bot
added
dependencies
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough첨부 목록 조회가 제한된 동시성, downstream timeout, 전체 예산으로 상태를 갱신합니다. 실패한 행은 기존 상태를 유지합니다. 응답에서 Changes첨부 상태 갱신 개선
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AttachmentRoute
participant refreshAttachmentStatuses
participant Clearfolio
participant AttachmentDatabase
Client->>AttachmentRoute: 첨부 목록 요청
AttachmentRoute->>AttachmentDatabase: jobId 포함 첨부 행 조회
AttachmentRoute->>refreshAttachmentStatuses: PENDING/RUNNING 행 전달
refreshAttachmentStatuses->>Clearfolio: 제한된 동시성으로 상태 조회
Clearfolio-->>refreshAttachmentStatuses: 상태 또는 오류 반환
refreshAttachmentStatuses->>AttachmentDatabase: 변경된 상태 저장
refreshAttachmentStatuses-->>AttachmentRoute: 갱신 카운터 반환
AttachmentRoute-->>Client: jobId 제거 응답 반환
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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으로 업데이트했습니다.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Reviewed: minor hono bump 4.12.32 -> 4.13.0, package.json + lockfile only, integrity hash consistent. Remediates CVE-2026-69207 flagged by trivy-fs on every open PR.
|
This pull request was built based on a group rule. Closing it will not ignore any of these versions in future pull requests. To ignore these dependencies, configure ignore rules in dependabot.yml |
…nges - cloud-sync.js parseMsProjectXml: keep develop's bounded linear scan (already merged via #386), superseding this branch's indexOf variant. - .trivyignore GHSA-frvp-7c67-39w9 removed: the hono vulnerability is fixed properly by upgrading hono (PR #419), not by suppressing the scanner. - CHANGELOG: fold the date-format optimization into the canonical Unreleased section instead of duplicated top-of-file headers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
|
@dependabot recreate |
99dc57b to
319150f
Compare
![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 head73603cd4cc4f0247d10002764bb5de4b54d261f8. -
Head SHA:
73603cd4cc4f0247d10002764bb5de4b54d261f8 -
Workflow run: 30899603557
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
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["Test (3 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (3 files)"]
R2 --> V2["targeted test run"]
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 (6 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (6 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test (3 files)"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test (3 files)"]
R2 --> V2["targeted test run"]
|
Pull request was converted to draft
Pull request was closed
|
This pull request was built based on a group rule. Closing it will not ignore any of these versions in future pull requests. To ignore these dependencies, configure ignore rules in dependabot.yml |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/attachment_status.mjs (1)
241-249: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External
CLEARFOLIO_URL에 HTTPS를 강제하십시오.
server/clearfolio.mjs는 URL scheme을 검증하지 않습니다.CLEARFOLIO_URL이http://이면 인증 헤더가 평문으로 전송되고, 네트워크 공격자가 상태 응답을 변경할 수 있습니다. 시작 시 비어 있지 않은 URL은https:만 허용하십시오. 빈 값의 mock mode는 유지하십시오.🤖 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 `@server/attachment_status.mjs` around lines 241 - 249, Update the CLEARFOLIO_URL initialization and validation in the clearfolio configuration flow to reject any non-empty URL whose scheme is not HTTPS. Preserve the existing behavior where an empty CLEARFOLIO_URL enables mock mode, and perform this validation during startup before requests are made.
🤖 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 `@server/clearfolio.mjs`:
- Around line 121-123: Update server/clearfolio.mjs lines 121-123 in the status
response handling to throw a fixed error when data.status is missing, removing
the 'FAILED' fallback so stale state is preserved. Update
tests/unit/clearfolio-status-signal.test.mjs lines 41-46 to verify that an empty
successful payload causes jobStatus to reject.
---
Outside diff comments:
In `@server/attachment_status.mjs`:
- Around line 241-249: Update the CLEARFOLIO_URL initialization and validation
in the clearfolio configuration flow to reject any non-empty URL whose scheme is
not HTTPS. Preserve the existing behavior where an empty CLEARFOLIO_URL enables
mock mode, and perform this validation during startup before requests are made.
🪄 Autofix
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: 08c2ee01-acb4-4f93-8308-1f8f63d5fb24
📒 Files selected for processing (8)
package.jsonserver/app.mjsserver/attachment_status.mjsserver/clearfolio.mjstests/api/attachment-status.test.mjstests/unit/attachment-status.test.mjstests/unit/clearfolio-status-signal.test.mjstests/unit/coverage-script-contract.test.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/api/attachment-status.test.mjs
- server/app.mjs
- tests/unit/attachment-status.test.mjs
| const data = await res.json().catch(() => ({})); | ||
| if (!res.ok) throw new Error(`clearfolio status failed (${res.status})`); | ||
| return data.status || 'FAILED'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
누락된 Clearfolio 상태를 terminal FAILED로 변환하지 마십시오.
성공 HTTP 응답에 status가 없으면 downstream payload 검증이 실패한 것입니다. 현재 구현은 이를 유효한 FAILED 상태로 변환하고, refresh worker가 기존 상태를 덮어쓰게 합니다. 오류를 throw하여 failed를 집계하고 stale 상태를 유지하십시오.
server/clearfolio.mjs#L121-L123:data.status가 없으면 고정된 오류를 throw하고 fallback 반환을 제거하십시오.tests/unit/clearfolio-status-signal.test.mjs#L41-L46: 빈 성공 payload가jobStatusrejection을 발생시키는지 검증하십시오.
📍 Affects 2 files
server/clearfolio.mjs#L121-L123(this comment)tests/unit/clearfolio-status-signal.test.mjs#L41-L46
🤖 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 `@server/clearfolio.mjs` around lines 121 - 123, Update server/clearfolio.mjs
lines 121-123 in the status response handling to throw a fixed error when
data.status is missing, removing the 'FAILED' fallback so stale state is
preserved. Update tests/unit/clearfolio-status-signal.test.mjs lines 41-46 to
verify that an empty successful payload causes jobStatus to reject.
|
@coderabbitai review |
✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review reconfirmed for the Hono 4.13.0 security update and bounded attachment-status refresh. The change removes the per-row lookup, enforces bounded concurrency and hard timeouts, isolates downstream and write failures, avoids exposing internal identifiers, adds operational metrics, preserves npm lock authority, documents the behavior, and includes focused regression and full new-module coverage evidence. No actionable finding remains.
1 participant
Summary
This bot-authored pull request combines the Hono security update with a buyer-visible attachment-list reliability improvement:
honofrom 4.12.32 to 4.13.0, remediating CVE-2026-69207;SELECT job_idlookup;test:coverageitself produce Istanbul JSON so central exact-head review cannot execute tests without coverage evidence;The refresh scheduler is isolated in
server/attachment_status.mjs, independent of Hono and SQLite, so it can be reused by a future standalone service adapter.Regression and quality evidence
Focused coverage proves:
server/attachment_status.mjshas 100% statement, branch, and function coverage.Verified evidence
One-shot exact-tree run
30902156688completed successfully and executed:npm run test:unit;npm run test:api;npm run coveragewith a non-emptycoverage/coverage-final.json;server/attachment_status.mjs;git diff --check.The temporary repair workflow and script removed themselves before the final product commit. Current-head repository and central required checks must still complete, and an independent current-head approval is required before merge.
Release note
CHANGELOG.mdrecords the bounded refresh, timeout, partial-failure, identifier-hiding, and operational-metric behavior under Unreleased.Closes #408. Supersedes #420.