Skip to content

fix(clearfolio): fail closed on production configuration - #493

Open
seonghobae wants to merge 31 commits into
developfrom
fix/clearfolio-production-configuration
Open

fix(clearfolio): fail closed on production configuration#493
seonghobae wants to merge 31 commits into
developfrom
fix/clearfolio-production-configuration

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Buyer and security impact

Refs #489. ScopeWeave no longer treats an absent Clearfolio integration as a successful production conversion service. The in-memory adapter is explicit development behavior; production conversion/viewing fails closed when the provider boundary is not configured, without disabling the planner. Tenant-signed provider calls refuse redirects, and browser artifact authority remains confined to the configured Clearfolio origin until a separately reviewed cross-origin allowlist exists.

Exact current state

  • protected live base: develop@2c328875e00e86537df3e965170be80532571cad;
  • exact contributor head: 78f8b557cd2b9cab238af72304f0ed42e1557759;
  • branch: fix/clearfolio-production-configuration;
  • current synthetic PR merge: 5c7914b7cbdd91ea7062c9b2b372a83e0281afd9;
  • PR is Ready and mechanically mergeable at the latest fresh refetch;
  • current reconciliation commit 78f8b557... preserves this bounded Clearfolio root slice while reconciling the protected Playwright 1.62.1 dependency update from current develop.

Protected changes inherited from develop are not claimed as Clearfolio feature work. Any head/base/review movement invalidates revision-sensitive evidence until freshly revalidated.

Configuration, redirect, and artifact authority contract

  • mock conversion exists only when SCOPEWEAVE_DEV=1 and no provider URL is configured;
  • unconfigured production operations fail with clearfolio_not_configured and expose no mock artifact route;
  • provider configuration is a canonical root service origin without credentials, path, query, fragment, unsupported scheme, or remote HTTP;
  • loopback HTTP is limited to explicit development mode, including [::1];
  • CLEARFOLIO_HMAC_SECRET is required with a provider and must contain at least 32 non-whitespace characters;
  • submit, status, and artifact-link provider fetches use redirect: "error", so tenant HMAC headers are not replayed onto provider-supplied redirects;
  • artifact URLs must resolve to the configured Clearfolio origin and contain no URL credentials or fragment; token-free foreign HTTPS, protocol-relative foreign links, credential-bearing same-origin links, fragmented links, and cross-origin token links fail closed;
  • a same-origin artifactToken is translated into the trusted viewer route without transplanting it to another origin; and
  • this root slice deliberately does not invent a cross-origin artifact-host allowlist. [Commercial Gap] Ship a fail-closed, bounded Clearfolio production adapter #489 remains responsible for any reviewed allowlist plus remaining streaming response-size/media-type, provider request-budget, lifecycle, and recovery controls.

Review-driven repairs retained on the current head

Cursor review found three valid predecessor defects: default redirect following could replay tenant HMAC headers, token-free foreign artifact links could become an attachment-view open redirect, and a unit assertion encoded that unsafe foreign redirect behavior. Test-first commit b8ebe96fbb7254638eab372ad4ee09bac78fa637 changed the provider-contract regression before production repair; production commit 32e44e4820eba722e59febe84c7f37d7a20930b0 applied the bounded fix.

Fresh current review-thread inventory contains those three Cursor threads only. All are resolved after source/tests changed; two remain current-and-resolved and one is outdated-and-resolved. No unresolved inline review thread remains. The current submitted reviews are Cursor COMMENTED predecessor evidence and a Devin COMMENTED no-issues result; neither is a qualifying independent formal approval of exact head 78f8b557....

Current repository evidence

All six pull-request workflows associated with exact contributor head 78f8b557cd2b9cab238af72304f0ed42e1557759 are terminal GitHub-success:

  • Dependency Review 32400078219;
  • SAST Semgrep 32400078258;
  • Server Tests 32400078276;
  • OSV Scanner 32400079352;
  • Fuzz 32400078217; and
  • Security Scan 32400078266.

Server Tests is useful behavioral evidence, but not exact-contributor-head merge authority under the current repository control. unit-and-api job 96525887797 explicitly fetched and checked out refs/pull/493/merge and logged HEAD is now at 5c7914b Merge 78f8b557... into 2c328875... rather than executing immutable contributor head 78f8b557.... Its broad unit/API suite, including Clearfolio production-configuration and provider-link regressions, passed, as did cloud-e2e job 96525888114; npm ci reported zero vulnerabilities. The result remains synthetic-merge behavioral evidence until #523's repository exact-head checkout/coverage control is protected-shipped and this then-current branch is freshly re-evidenced.

Centrally reusable SAST/Security exact-head evidence remains separately owner-controlled by ContextualWisdomLab/.github#1222; a central green status is not promoted to contributor-head authority while that control defect remains open. No predecessor, synthetic-only, status-only, author-only, or model-only result transfers into merge authority.

Stack responsibility

This PR is the Clearfolio #489 root slice. Child #498 and later Clearfolio descendants must be reconciled semantically against exact current parent 78f8b557cd2b9cab238af72304f0ed42e1557759 before integration; historical parent SHAs in child PR bodies are not authority. Preserve unique child provider-boundary, artifact-origin, readiness, and operator-surface work rather than copying or parallelizing the root boundary.

Standards and traceability

docs/doctoring/clearfolio-production-configuration.md records the trust boundary, redirect prohibition, same-origin artifact authority, explicit future-allowlist boundary, rollback, executable regressions, and APA 7 references to the WHATWG URL Standard, OWASP SSRF Prevention guidance, and NIST SSDF 1.1.

Merge gate

Do not merge or enable auto-merge until the unchanged exact head remains reconciled to the freshly resolved protected base, #523's repository exact-head evidence control and the applicable central reusable exact-head security/SAST repair are protected-shipped, every applicable repository and organization CI/browser/owned-coverage/docstring/CodeQL/SAST/security/dependency/supply-chain/package/SBOM/provenance/recovery/required-review gate is substantively terminal-passing on the exact contributor revision, valid unresolved findings are zero, and a genuine qualifying independent current-head/last-push approval exists under live protection/rulesets.

Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, model-only, rate-limited, or infrastructure-only evidence is non-authorizing.


Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6710fc78-5656-40a0-ade2-a63db33d11ab

📥 Commits

Reviewing files that changed from the base of the PR and between d122ca7 and 78f8b55.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • docs/doctoring/clearfolio-production-configuration.md
  • server/clearfolio.mjs
  • tests/unit/clearfolio-status-signal.test.mjs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Clearfolio 설정 검증과 개발 모드 제한을 추가했습니다. 프로덕션 요청은 검증된 HTTPS origin과 HMAC secret을 사용합니다. redirect 추적을 차단하고, artifact 링크는 동일 origin·무자격 증명·무fragment 조건을 충족해야 합니다. 관련 문서와 테스트도 갱신했습니다.

Changes

Clearfolio 보안 경계

Layer / File(s) Summary
설정 검증 및 개발 모드 경계
server/clearfolio.mjs, tests/unit/clearfolio-adapter-mock-hmac.test.mjs, docs/deploy.md, tests/api/attachment-status.test.mjs
Clearfolio URL과 HMAC secret을 검증합니다. 명시적 개발 모드에서만 인메모리 mock과 loopback HTTP를 허용합니다. 프로덕션에서 provider가 없으면 clearfolio_not_configured로 실패합니다.
HMAC 인증 provider 요청
server/clearfolio.mjs, tests/unit/clearfolio-adapter-mock-hmac.test.mjs, tests/unit/clearfolio-status-signal.test.mjs
작업 제출, 상태 조회 및 artifact 요청이 검증된 provider URL과 HMAC tenant headers를 사용합니다. 모든 요청은 redirect 응답을 오류로 처리합니다.
Artifact URL origin 경계
server/clearfolio.mjs, tests/unit/clearfolio-status-signal.test.mjs
외부 origin, credentials 포함 URL 및 fragment 포함 URL을 거부합니다. 동일 origin의 artifactToken만 내부 viewer 경로로 변환합니다.
보안 동작 및 릴리스 기록 문서화
docs/doctoring/clearfolio-production-configuration.md, docs/deploy.md, CHANGELOG.md
프로덕션 경계, 테스트 범위, 보안 근거, 롤백 범위 및 관련 릴리스 변경을 기록합니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🔵 Low · up to 78f8b

This change makes unconfigured production Clearfolio operations fail closed and blocks unsafe provider redirects and artifact links. Repository checks are passing and no actionable defect remains in the documentation, but an independent approval for the exact current head is still required before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Scopeweave
  participant Clearfolio
  participant ArtifactViewer
  Scopeweave->>Clearfolio: submit job with signed tenant claims
  Clearfolio-->>Scopeweave: job status and signed artifact URL
  Scopeweave->>Scopeweave: validate origin, credentials, and fragment
  Scopeweave->>ArtifactViewer: open same-origin viewer URL with artifact token
  Scopeweave-->>Scopeweave: reject redirects and invalid artifact URLs
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 프로덕션 설정이 없을 때 Clearfolio가 fail-closed 동작을 수행하도록 변경한 주요 내용을 정확하게 요약합니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/clearfolio-production-configuration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 14, 2026 15:56

Copy link
Copy Markdown
Contributor Author

@opencode-agent review

Please submit an independent formal review for exact current head c8da068538e5f5032136f0a39a68ab387edc3646. All six repository-native workflows are terminal-success on this exact head. Verify explicit development-only mock semantics, production fail-closed configuration, provider URL/HMAC boundary, cross-origin artifact-token isolation, sanitized downstream failures, and that the seven-file slice does not weaken the integrated attachment refresh/session/auth behavior. Do not transfer predecessor-head evidence, mutate the branch, merge, or bypass the required last-push approval rule.

@seonghobae
seonghobae enabled auto-merge (squash) August 14, 2026 16:24

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae marked this pull request as draft August 14, 2026 16:29
auto-merge was automatically disabled August 14, 2026 16:29

Pull request was converted to draft

@seonghobae
seonghobae marked this pull request as ready for review August 14, 2026 16:30

Copy link
Copy Markdown
Contributor Author

@opencode-agent
@cwl-noema-review

Copy link
Copy Markdown
Contributor Author

@opencode-agent

@seonghobae
seonghobae enabled auto-merge (squash) August 14, 2026 18:23

Copy link
Copy Markdown
Contributor Author

@opencode-agent
@cwl-noema-review

Retry independent review for exact current head c8da068538e5f5032136f0a39a68ab387edc3646 against live develop@b88e66e81e9701404d29a0f5de4f58573ceee14f. All six repository-native workflows are terminal-success. Verify production Clearfolio is fail-closed unless explicitly configured, the mock is development-only, URL/HMAC and artifact-token origin boundaries are fail-closed, downstream errors remain sanitized, and the seven-file slice does not weaken integrated attachment/session/auth behavior. Publish current-head review evidence only; do not mutate or merge.

@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

cursor[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

@opencode-agent Review-only request for exact current head be5a66b88e067fe60b3672e127be14781214388f; do not mutate, merge, or reuse predecessor evidence. Re-evaluate the Cursor findings from predecessor d122ca75… against current source: redirect following and cross-origin artifact-link findings are addressed and all three inline threads are resolved. Bind the verdict to live develop@44e7903cf8891c65410f7fc6ca5144de3fdb5185 and current exact-head evidence only. Keep #523/central Strix evidence-integrity prerequisites separate from semantic source review, and do not fabricate source findings from provider-unavailable infrastructure evidence.

Reconcile PR #493 onto current protected develop, retaining #545 OpenCode/NIM configuration and release-note regressions while preserving the bounded Clearfolio production configuration and provider-origin hardening slice.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current head 09ae6d00f5df68f5042f1decb2bab6a859caf545. Treat predecessor reviews as historical. Focus on fail-closed provider configuration, redirect/HMAC replay resistance, artifact-origin authority, credentials/fragments/protocol-relative URL handling, development-only mock behavior, stable error non-disclosure, and preservation of protected-develop behavior. Review only; do not infer approval.

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

@seonghobae I requested a review for the current PR state. The review will assess the specified security boundaries and protected-develop behavior. It will not infer approval.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

seonghobae added a commit that referenced this pull request Aug 19, 2026
Rebuild the child from the current #493 parent tree, preserve only the bounded provider transport/JSON response slice, and retain protected adaptive-orchestrator attribution changes without widening the stack.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@opencode-agent opencode-agent Bot added priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug Defect or incorrect behavior labels Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 프로덕션 Clearfolio 설정 누락 시 fail-closed 동작을 정확하고 간결하게 설명하며 변경 사항의 주요 목적과 일치합니다.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@opencode-agent review

Review-only request for exact current head 78f8b557cd2b9cab238af72304f0ed42e1557759 against live protected develop@2c328875e00e86537df3e965170be80532571cad. The previous OpenCode request targeted predecessor c8da068... and is not current evidence. Bind any formal verdict to this exact head/base. Re-verify explicit development-only mock semantics, production fail-closed configuration, canonical Clearfolio URL/HMAC boundary, redirect refusal on all tenant-signed provider calls, same-origin artifact authority/credential+fragment rejection, and preservation of the planner when the capability is unavailable. The three predecessor Cursor findings are resolved on current source. Do not mutate source, rebase, merge, or weaken gates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant