Skip to content

fix(clearfolio): fail closed on provider redirects and unreviewed hosts - #536

Closed
cursor[bot] wants to merge 22 commits into
developfrom
cursor/bc-b1c7e1f9-abca-4c2c-98bd-c04e9bb18572-3e4e
Closed

fix(clearfolio): fail closed on provider redirects and unreviewed hosts#536
cursor[bot] wants to merge 22 commits into
developfrom
cursor/bc-b1c7e1f9-abca-4c2c-98bd-c04e9bb18572-3e4e

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Buyer and security impact

Successor of #493 (d122ca75). Keep that head unmerged. This branch retains the production fail-closed Clearfolio contract and closes two buyer-visible holes that #493 still left open:

  1. Provider fetch now uses redirect: 'error', so a 3xx from the pinned origin cannot replay tenant HMAC headers onto another host.
  2. Attachment-view links must share the configured Clearfolio origin. Token-free CDN/protocol-relative/credentialed/fragmented URLs fail closed instead of becoming an unreviewed 302.

HMAC secrets also trim surrounding whitespace (including a secret-file newline) before length-check and signing.

Exact current scope

  • base: develop@28420da358f57be5e85be3660251e39b85e1cc94
  • head includes fix(clearfolio): fail closed on production configuration #493 plus 5c2f73d
  • changed paths stay on the Clearfolio adapter, its unit tests, deploy/doctoring/CHANGELOG, and a short ARCHITECTURE decision
  • no database schema, workflow, scanner suppression, or protected-branch policy change

Verification

Local npm run test:unit and npm run test:api passed on this head, including the new redirect, remote-HTTP-in-dev, HMAC-newline, and unreviewed-host cases.

docs/doctoring/clearfolio-production-configuration.md records the redirect and same-origin rules, rollback, remaining #489 gaps (reviewed artifact-host allowlist, streaming body/media-type limits, request budget), and APA 7 references to the WHATWG URL Standard, OWASP SSRF Prevention guidance, and NIST SSDF 1.1.

Do not self-approve. Do not merge #493. Re-evaluate this unchanged head against live required checks before integration.

Open in Web View Automation 

seonghobae and others added 22 commits August 14, 2026 23:17
Stop following Clearfolio 3xx responses so tenant HMAC headers cannot be
replayed onto another origin. Reject token-free cross-origin, protocol-
relative, credentialed, and fragmented artifact links instead of issuing
an unreviewed attachment-view 302. Trim secret-file whitespace before
HMAC signing and prove remote HTTP stays rejected in development.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor
cursor Bot requested a review from seonghobae August 16, 2026 16:00

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do not land beside the origin-policy stack

This draft targets develop and overlaps Clearfolio redirect/host trust that #498 + #538 already own on the stacked parent fix/clearfolio-provider-response-boundary.

The current artifact-origin landing vehicle is #538 0c193aa28e730e5c1a18e7d740637f34efbbd95d. Landing this unstacked develop child would fork the adapter contract and skip the provider-response parent.

Do not merge. Do not treat this as a substitute for #538. If #493-shaped transport work is still needed, rebase it onto the live parent after #498 rather than widening develop in parallel.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Copy link
Copy Markdown
Contributor

Closing as superseded after fresh comparison with the current Clearfolio stack. #498 owns the stronger bounded provider transport/redirect boundary, and #538 owns the reviewed artifact-origin policy on top of that parent. This draft forks from #493/develop and would duplicate those production surfaces rather than extend the canonical stack.

The one extra CLEARFOLIO_HMAC_SECRET.trim() behavior is intentionally not preserved: silently stripping configured cryptographic key bytes changes key material and is not part of the current Clearfolio interoperability contract. Any newline handling should be specified and tested at the secret-loading contract rather than introduced incidentally in this overlapping adapter PR. No other unique production behavior here needs to survive #498 + #538.

@seonghobae seonghobae closed this Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants