From 9f7de2968c186e261adb1beae38b52dce2dd1de7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 14:46:21 +0900 Subject: [PATCH 01/12] test(clearfolio): require explicit artifact origin trust --- package.json | 4 +- .../unit/clearfolio-artifact-origin.test.mjs | 111 ++++++++++++++++++ 2 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 tests/unit/clearfolio-artifact-origin.test.mjs diff --git a/package.json b/package.json index 4134834c..ce17f8ce 100644 --- a/package.json +++ b/package.json @@ -13,9 +13,9 @@ "coverage": "npm run test:coverage", "server": "node server/server.mjs", "test:api": "node tests/api/auth-secret.test.mjs && node tests/api/smoke.mjs && node tests/api/ratelimit.test.mjs && node tests/api/attachment-status.test.mjs && node tests/api/session-revocation.test.mjs", - "test:unit": "node tests/unit/analytics.test.mjs && node tests/unit/cpm.test.mjs && node tests/unit/baseline-compare.test.mjs && node tests/unit/workload.test.mjs && node tests/unit/cost-evm.test.mjs && node tests/unit/msproject.test.mjs && node tests/unit/auth-password.test.mjs && node tests/unit/editor-unsaved.test.mjs && node tests/unit/static-coverage-evidence.test.mjs && node tests/unit/dep-types.test.mjs && node tests/unit/weekly-report.test.mjs && node tests/unit/clearfolio.test.mjs && node tests/unit/clearfolio-adapter-mock-hmac.test.mjs && node tests/unit/clearfolio-provider-boundary.test.mjs && node tests/unit/sprint-stats.test.mjs && node tests/unit/burndown.test.mjs && node tests/unit/pm-analysis.test.mjs && node tests/unit/cloud-sync-security.test.mjs && node tests/unit/attachment-status.test.mjs && node tests/unit/clearfolio-status-signal.test.mjs && node tests/unit/coverage-script-contract.test.mjs", + "test:unit": "node tests/unit/analytics.test.mjs && node tests/unit/cpm.test.mjs && node tests/unit/baseline-compare.test.mjs && node tests/unit/workload.test.mjs && node tests/unit/cost-evm.test.mjs && node tests/unit/msproject.test.mjs && node tests/unit/auth-password.test.mjs && node tests/unit/editor-unsaved.test.mjs && node tests/unit/static-coverage-evidence.test.mjs && node tests/unit/dep-types.test.mjs && node tests/unit/weekly-report.test.mjs && node tests/unit/clearfolio.test.mjs && node tests/unit/clearfolio-adapter-mock-hmac.test.mjs && node tests/unit/clearfolio-provider-boundary.test.mjs && node tests/unit/clearfolio-artifact-origin.test.mjs && node tests/unit/sprint-stats.test.mjs && node tests/unit/burndown.test.mjs && node tests/unit/pm-analysis.test.mjs && node tests/unit/cloud-sync-security.test.mjs && node tests/unit/attachment-status.test.mjs && node tests/unit/clearfolio-status-signal.test.mjs && node tests/unit/coverage-script-contract.test.mjs", "test:coverage": "c8 --all --include=app.js --include=cloud-sync.js --include=scripts/ci/static_coverage_evidence.mjs --include=server/attachment_status.mjs --include=server/app.mjs --include=server/auth.mjs --include=server/clearfolio.mjs --reporter=json --reporter=json-summary npm run test:coverage:cases", - "test:coverage:cases": "node tests/unit/coverage-script-contract.test.mjs && node tests/unit/attachment-status.test.mjs && node tests/unit/clearfolio-status-signal.test.mjs && node tests/unit/clearfolio-adapter-mock-hmac.test.mjs && node tests/unit/clearfolio-provider-boundary.test.mjs && node tests/unit/msproject.test.mjs && node tests/unit/auth-password.test.mjs && node tests/unit/editor-unsaved.test.mjs && node tests/unit/static-coverage-evidence.test.mjs && npm run test:api", + "test:coverage:cases": "node tests/unit/coverage-script-contract.test.mjs && node tests/unit/attachment-status.test.mjs && node tests/unit/clearfolio-status-signal.test.mjs && node tests/unit/clearfolio-adapter-mock-hmac.test.mjs && node tests/unit/clearfolio-provider-boundary.test.mjs && node tests/unit/clearfolio-artifact-origin.test.mjs && node tests/unit/msproject.test.mjs && node tests/unit/auth-password.test.mjs && node tests/unit/editor-unsaved.test.mjs && node tests/unit/static-coverage-evidence.test.mjs && npm run test:api", "test:e2e": "playwright test", "test:e2e:headed": "playwright test --headed", "test:e2e:cloud": "playwright install chromium && playwright test tests/e2e/cloud.spec.js", diff --git a/tests/unit/clearfolio-artifact-origin.test.mjs b/tests/unit/clearfolio-artifact-origin.test.mjs new file mode 100644 index 00000000..2094c39f --- /dev/null +++ b/tests/unit/clearfolio-artifact-origin.test.mjs @@ -0,0 +1,111 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +const HMAC_SECRET = 'clearfolio-shared-secret-32-bytes!!'; +const originalFetch = globalThis.fetch; +let importSequence = 0; +let fetchCalls = 0; +let artifactPayload = { artifactUrl: 'https://clearfolio.example/file.pdf' }; + +globalThis.fetch = async () => { + fetchCalls += 1; + return new Response(JSON.stringify(artifactPayload), { + status: 200, + headers: { 'content-type': 'application/json; charset=utf-8' }, + }); +}; + +async function loadAdapter(artifactOrigins) { + process.env.CLEARFOLIO_URL = 'https://clearfolio.example'; + process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; + delete process.env.SCOPEWEAVE_DEV; + if (artifactOrigins === undefined) delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + else process.env.CLEARFOLIO_ARTIFACT_ORIGINS = artifactOrigins; + importSequence += 1; + return import(`../../server/clearfolio.mjs?artifact-origin-policy=${importSequence}`); +} + +async function resolveArtifact(link, artifactOrigins) { + artifactPayload = { artifactUrl: link }; + const { artifactUrl } = await loadAdapter(artifactOrigins); + return artifactUrl(4, 5, 'job-1'); +} + +test.after(() => { + globalThis.fetch = originalFetch; + delete process.env.CLEARFOLIO_URL; + delete process.env.CLEARFOLIO_HMAC_SECRET; + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + delete process.env.SCOPEWEAVE_DEV; +}); + +test('artifact URLs default to the configured Clearfolio origin', async () => { + await assert.rejects( + () => resolveArtifact('https://cdn.example/file.pdf', undefined), + /clearfolio artifact-link response invalid/, + ); + assert.equal( + await resolveArtifact('/signed/file.pdf', undefined), + 'https://clearfolio.example/signed/file.pdf', + ); +}); + +test('explicit HTTPS artifact origins are exact scheme-host-port allowlist entries', async () => { + assert.equal( + await resolveArtifact('https://cdn.example/file.pdf', ' https://cdn.example '), + 'https://cdn.example/file.pdf', + ); + assert.equal( + await resolveArtifact( + 'https://cdn.example/file.pdf?artifactToken=remote%20token', + 'https://cdn.example', + ), + 'https://cdn.example/file.pdf?artifactToken=remote%20token', + 'an allowlisted cross-origin token remains bound to the returned origin', + ); + await assert.rejects( + () => resolveArtifact('https://cdn.example/file.pdf', 'https://cdn.example:8443'), + /clearfolio artifact-link response invalid/, + 'an allowlist entry with a different port is a different origin', + ); +}); + +test('artifact links reject credentials and fragments even on trusted origins', async () => { + for (const link of [ + 'https://user:password@clearfolio.example/file.pdf', + 'https://clearfolio.example/file.pdf#secret-fragment', + 'https://user:password@cdn.example/file.pdf', + 'https://cdn.example/file.pdf#secret-fragment', + ]) { + await assert.rejects( + () => resolveArtifact(link, 'https://cdn.example'), + /clearfolio artifact-link response invalid/, + ); + } +}); + +test('artifact origin configuration fails closed before provider transport', async () => { + const invalidConfigurations = [ + 'http://cdn.example', + 'https://user:password@cdn.example', + 'https://cdn.example/path', + 'https://cdn.example?query=1', + 'https://cdn.example#fragment', + 'not a URL', + 'https://cdn.example,', + ]; + + for (const artifactOrigins of invalidConfigurations) { + const before = fetchCalls; + artifactPayload = { artifactUrl: 'https://cdn.example/file.pdf' }; + const { artifactUrl } = await loadAdapter(artifactOrigins); + await assert.rejects( + () => artifactUrl(4, 5, 'job-1'), + (error) => { + assert.equal(error.code, 'clearfolio_artifact_origins_invalid'); + return true; + }, + ); + assert.equal(fetchCalls, before, `invalid allowlist ${artifactOrigins} never reaches provider transport`); + } +}); From ce4e7c574a5a02134e76d2fb03d69017952b7635 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 14:54:28 +0900 Subject: [PATCH 02/12] fix(clearfolio): enforce artifact origin trust --- server/clearfolio.mjs | 60 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/server/clearfolio.mjs b/server/clearfolio.mjs index c93be182..fd716524 100644 --- a/server/clearfolio.mjs +++ b/server/clearfolio.mjs @@ -122,6 +122,58 @@ function clearfolioConfiguration() { }; } +/** + * Build the exact set of origins trusted to host Clearfolio artifacts. + * + * The configured Clearfolio origin is always trusted. Additional origins are + * optional and must be comma-separated HTTPS origins with no credentials, + * path, query, or fragment. Canonical URL origins preserve exact scheme, host, + * and effective port identity while preventing string-prefix allowlist bypasses. + * + * @param {string} baseUrl - Validated Clearfolio provider origin. + * @returns {Set} Canonical origins accepted for artifact redirects. + * @throws {ClearfolioConfigurationError} If the optional allowlist is malformed or unsafe. + */ +function clearfolioArtifactOrigins(baseUrl) { + const trustedOrigins = new Set([new URL(baseUrl).origin]); + const configuredOrigins = process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + if (configuredOrigins === undefined) return trustedOrigins; + + const entries = String(configuredOrigins).split(','); + if (entries.some((entry) => entry.trim().length === 0)) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS must contain only comma-separated HTTPS origins.', + ); + } + + for (const entry of entries) { + let url; + try { + url = new URL(entry.trim()); + } catch { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS must contain only comma-separated HTTPS origins.', + ); + } + if ( + url.protocol !== 'https:' + || Boolean(url.username + url.password) + || url.pathname !== '/' + || Boolean(url.search) + || Boolean(url.hash) + ) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS must contain only comma-separated HTTPS origins.', + ); + } + trustedOrigins.add(url.origin); + } + return trustedOrigins; +} + /** * Sign tenant claims using the Clearfolio HMAC interoperability contract. * @@ -455,6 +507,7 @@ export async function artifactUrl(orgId, userId, jobId) { const canonicalJobId = validateJobId(jobId); const configuration = clearfolioConfiguration(); if (configuration.mock) return `/api/mock-clearfolio/${encodeURIComponent(canonicalJobId)}`; + const trustedArtifactOrigins = clearfolioArtifactOrigins(configuration.baseUrl); let res; try { res = await fetch(`${configuration.baseUrl}/api/v1/viewer/${encodeURIComponent(canonicalJobId)}/artifact-links`, { @@ -486,6 +539,13 @@ export async function artifactUrl(orgId, userId, jobId) { if (url.protocol !== 'https:' && !allowsHttp) { throw new Error('clearfolio artifact-link response invalid'); } + if ( + Boolean(url.username + url.password) + || Boolean(url.hash) + || !trustedArtifactOrigins.has(url.origin) + ) { + throw new Error('clearfolio artifact-link response invalid'); + } const token = url.searchParams.get('artifactToken'); if (token && url.origin === clearfolioUrl.origin) { From 462f6594c4d326152146a7403896381c8e34a270 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 14:55:29 +0900 Subject: [PATCH 03/12] docs(changelog): record artifact origin trust --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fce05d63..3501968e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 required a canonical signed production origin, rejected ambiguous provider URL components, and prevented cross-origin artifact tokens from being transplanted into the trusted Clearfolio viewer URL. +- Restricted hosted Clearfolio artifact redirects to the provider origin by + default; optional `CLEARFOLIO_ARTIFACT_ORIGINS` entries must be exact HTTPS + origins, so scheme/host/port changes, credentials, fragments, and unapproved + cross-origin links fail closed while approved cross-origin tokens remain bound + to the origin that issued them. - Bounded hosted Clearfolio calls to non-redirecting 15-second requests and 256 KiB streamed JSON responses, composed caller cancellation with the provider budget, and validated document metadata/bytes and provider job IDs From 865fec5a51cb0230f2e2830e0ee3dd15e534a901 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 14:55:53 +0900 Subject: [PATCH 04/12] docs(clearfolio): record artifact origin trust boundary --- .../clearfolio-artifact-origin-trust.md | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 docs/doctoring/clearfolio-artifact-origin-trust.md diff --git a/docs/doctoring/clearfolio-artifact-origin-trust.md b/docs/doctoring/clearfolio-artifact-origin-trust.md new file mode 100644 index 00000000..38507d3d --- /dev/null +++ b/docs/doctoring/clearfolio-artifact-origin-trust.md @@ -0,0 +1,57 @@ +# Clearfolio artifact-origin trust boundary + +## Decision + +ScopeWeave treats every artifact link returned by Clearfolio as untrusted provider data. The configured `CLEARFOLIO_URL` origin is the default artifact trust boundary. A production operator may add reviewed CDN or object-storage origins through `CLEARFOLIO_ARTIFACT_ORIGINS`, but each entry must be an origin only: HTTPS scheme, host, and optional non-default port, with no credentials, path, query, fragment, or empty comma-separated entry. + +This slice is deliberately narrower than the complete Clearfolio production-adapter program in issue #489. It does not claim that provider DNS/IP authorization, artifact content validation, retention, or all operational acceptance work is complete. It closes the redirect-origin and token-confusion boundary on top of the provider-response controls owned by the parent PR. + +## Why exact origins + +RFC 6454 defines an origin around scheme, host, and port. Comparing canonical URL origins therefore keeps `https://cdn.example`, `https://cdn.example:8443`, and HTTP variants in distinct trust domains instead of relying on string-prefix matching. The WHATWG URL Standard supplies the parser and serialization semantics used by Node's `URL` implementation, including explicit username/password and fragment components. + +The adapter uses a positive allowlist rather than accepting any syntactically valid HTTPS URL. OWASP's SSRF guidance recommends allowlisting known destinations and disabling or tightly validating redirects when the intended service set is known. Although ScopeWeave is redirecting a browser to a provider-selected artifact rather than issuing a second server-side fetch, the same positive-trust principle prevents an untrusted provider response from turning the application into an arbitrary external redirector. + +## Runtime contract + +1. `CLEARFOLIO_ARTIFACT_ORIGINS` is optional. When absent, only the validated Clearfolio provider origin is trusted. +2. When present, the value is a comma-separated list of canonicalizable HTTPS origins. Whitespace around entries is ignored; empty entries are rejected. +3. Any malformed entry, HTTP entry, URL credential, path, query, or fragment produces `ClearfolioConfigurationError` with stable code `clearfolio_artifact_origins_invalid` before the artifact-link provider request is sent. +4. Provider-returned artifact URLs must still satisfy the existing HTTP/HTTPS and downgrade rules, must contain no credentials or fragment, and must resolve to the provider origin or an explicitly configured artifact origin. +5. A same-origin `artifactToken` may be translated into the trusted Clearfolio viewer route. A token on an approved cross-origin artifact URL remains on that returned URL; ScopeWeave never transplants it into the provider-origin viewer. +6. Exact origin comparison includes the effective port. Approving `https://cdn.example:8443` does not approve `https://cdn.example`. + +## Operator action + +If Clearfolio returns artifacts from a separate reviewed CDN or object-storage service, configure only that service origin, for example: + +```text +CLEARFOLIO_ARTIFACT_ORIGINS=https://artifacts.example.com,https://archive.example.com:8443 +``` + +Do not place signed paths, object keys, tokens, credentials, query strings, or fragments in this setting. If no cross-origin artifact service is required, leave the variable unset; the provider origin remains the least-privilege default. + +## Verification contract + +`tests/unit/clearfolio-artifact-origin.test.mjs` exercises the production adapter with real `URL` parsing and a bounded mocked provider response. It proves: + +- cross-origin HTTPS artifacts fail by default; +- same-origin relative artifacts continue to resolve against the provider; +- an explicitly approved origin succeeds only for the same scheme/host/port identity; +- approved cross-origin `artifactToken` values remain on the approved origin; +- credentials and fragments are rejected on provider and approved origins; +- malformed, HTTP, credentialed, path-, query-, fragment-bearing, and empty-entry configuration fails before any provider transport. + +The test is registered in both the normal unit suite and the production coverage cases so changes to this boundary cannot silently bypass repository coverage evidence. + +## Failure and rollback + +Configuration failure is fail-closed and limited to the artifact-link capability; it does not broaden trust or silently fall back to arbitrary URLs. Rollback removes the additional-origin feature and returns to provider-origin-only artifact redirects. Do not roll back by permitting arbitrary HTTPS destinations or by moving cross-origin tokens into a trusted same-origin viewer URL. + +## References + +Barth, A. (2011). *The web origin concept* (RFC 6454). Internet Engineering Task Force. https://doi.org/10.17487/RFC6454 + +Open Worldwide Application Security Project. (n.d.). *Server side request forgery prevention cheat sheet*. OWASP Cheat Sheet Series. Retrieved August 15, 2026, from https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html + +WHATWG. (2026). *URL standard*. https://url.spec.whatwg.org/ From e4d2f5f02294fe3e337e075ee05ad2afc2889aa6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 14:58:20 +0900 Subject: [PATCH 05/12] test(clearfolio): align legacy artifact origin contract --- tests/unit/clearfolio-status-signal.test.mjs | 37 ++++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/tests/unit/clearfolio-status-signal.test.mjs b/tests/unit/clearfolio-status-signal.test.mjs index 9ea164cb..70c54199 100644 --- a/tests/unit/clearfolio-status-signal.test.mjs +++ b/tests/unit/clearfolio-status-signal.test.mjs @@ -54,6 +54,7 @@ test.after(() => { globalThis.fetch = originalFetch; delete process.env.CLEARFOLIO_URL; delete process.env.CLEARFOLIO_HMAC_SECRET; + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; delete process.env.SCOPEWEAVE_DEV; }); @@ -244,22 +245,27 @@ test('artifactUrl validates links and never exposes transport or response text', 'same-origin artifact tokens may be translated into the trusted viewer route', ); - setResponse({ json: async () => ({ url: 'https://cdn.example/file.pdf' }) }); - assert.equal( - await artifactUrl(4, 5, 'job-1'), - 'https://cdn.example/file.pdf', - ); + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = 'https://cdn.example'; + try { + setResponse({ json: async () => ({ url: 'https://cdn.example/file.pdf' }) }); + assert.equal( + await artifactUrl(4, 5, 'job-1'), + 'https://cdn.example/file.pdf', + ); - setResponse({ - json: async () => ({ - signedUrl: 'https://cdn.example/file.pdf?artifactToken=token%20value', - }), - }); - assert.equal( - await artifactUrl(4, 5, 'job-1'), - 'https://cdn.example/file.pdf?artifactToken=token%20value', - 'a token from another origin is never transplanted into the trusted Clearfolio viewer', - ); + setResponse({ + json: async () => ({ + signedUrl: 'https://cdn.example/file.pdf?artifactToken=token%20value', + }), + }); + assert.equal( + await artifactUrl(4, 5, 'job-1'), + 'https://cdn.example/file.pdf?artifactToken=token%20value', + 'an explicitly trusted cross-origin token remains bound to its artifact origin', + ); + } finally { + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + } }); test('artifactUrl permits HTTP only for explicit loopback development', async () => { @@ -277,6 +283,7 @@ test('artifactUrl permits HTTP only for explicit loopback development', async () ); } finally { process.env.CLEARFOLIO_URL = 'https://clearfolio.example'; + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; delete process.env.SCOPEWEAVE_DEV; } }); From 0c193aa28e730e5c1a18e7d740637f34efbbd95d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:02:47 +0000 Subject: [PATCH 06/12] fix(clearfolio): require canonical artifact-origin allowlist entries Reject non-canonical CLEARFOLIO_ARTIFACT_ORIGINS lookalikes before provider transport, cover protocol-relative and userinfo-as-host redirects, and put the operator control in deploy/API docs so a reviewed CDN origin can be configured without guessing. Co-authored-by: Seongho Bae --- ARCHITECTURE.md | 4 +++ CHANGELOG.md | 3 ++- README.md | 5 +++- docs/api.md | 6 +++-- docs/deploy.md | 9 +++++++ .../clearfolio-artifact-origin-trust.md | 9 ++++--- .../clearfolio-production-configuration.md | 4 +-- .../clearfolio-provider-response-boundary.md | 2 +- server/clearfolio.mjs | 4 ++- .../unit/clearfolio-artifact-origin.test.mjs | 27 +++++++++++++++++++ 10 files changed, 61 insertions(+), 12 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3006c74b..4fa04b7f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -43,3 +43,7 @@ contain. - Kubernetes/IaC security coverage remains a follow-up design lane for any future `infra/` or container packaging surface. +- Clearfolio artifact redirects trust only the configured provider origin + unless `CLEARFOLIO_ARTIFACT_ORIGINS` lists additional exact HTTPS origins. + Cross-origin `artifactToken` values stay on the returned origin and are + never transplanted into the Clearfolio viewer. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ff3b34c..16a9fd37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 transplanted into the trusted Clearfolio viewer URL. - Restricted hosted Clearfolio artifact redirects to the provider origin by default; optional `CLEARFOLIO_ARTIFACT_ORIGINS` entries must be exact HTTPS - origins, so scheme/host/port changes, credentials, fragments, and unapproved + origins (`URL.origin` or that origin plus `/`), so scheme/host/port changes, + credentials, fragments, protocol-relative lookalikes, and unapproved cross-origin links fail closed while approved cross-origin tokens remain bound to the origin that issued them. - Bounded hosted Clearfolio calls to non-redirecting 15-second requests and diff --git a/README.md b/README.md index ea8bb77a..c0da0685 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,10 @@ Docker: set a **persistent** `SCOPEWEAVE_JWT_SECRET` first, then run `docker com | `OIDC_ISSUER/CLIENT_ID/CLIENT_SECRET/REDIRECT_URI` | Real SSO IdP (mock when unset) | | `STRIPE_SECRET_KEY` | Real checkout (mock URL when unset) | | `SCOPEWEAVE_RATE_LIMIT_MAX` (+`_WINDOW_MS`) | Opt-in per-IP rate limiting | -| `SCOPEWEAVE_DEV=1` | Dev-only endpoints (activate-pro) | +| `SCOPEWEAVE_DEV=1` | Dev-only endpoints (activate-pro, loopback Clearfolio HTTP, in-memory Clearfolio adapter when no provider URL exists). Never set in production. | +| `CLEARFOLIO_URL` | Production 산출물 viewer origin (HTTPS). Unset fails closed unless `SCOPEWEAVE_DEV=1`. | +| `CLEARFOLIO_HMAC_SECRET` | Tenant-claim HMAC secret (at least 32 non-whitespace characters) when a provider URL is set. | +| `CLEARFOLIO_ARTIFACT_ORIGINS` | Optional comma-separated exact HTTPS artifact origins. Unset trusts only `CLEARFOLIO_URL`. Empty or malformed values fail closed. | ## Verification diff --git a/docs/api.md b/docs/api.md index 1c668425..b3d51d31 100644 --- a/docs/api.md +++ b/docs/api.md @@ -90,8 +90,10 @@ credentials never reach the browser. HWP/HWPX are rejected (Clearfolio policy). | `GET` | `/api/projects/:id/attachments/:aid/view` | 302 → signed artifact URL (`?token=` for new-tab opens) | | `DELETE` | `/api/projects/:id/attachments/:aid` | Uploader or manage | -Env: `CLEARFOLIO_URL` (+ optional `CLEARFOLIO_HMAC_SECRET` for gateway-signed -tenant claims). Unset → a built-in mock converter (dev/test only). +Env: `CLEARFOLIO_URL` plus `CLEARFOLIO_HMAC_SECRET` for production conversion. +Optional `CLEARFOLIO_ARTIFACT_ORIGINS` adds reviewed HTTPS CDN/object-store +origins; unset trusts only the Clearfolio origin. An unset URL is not a +successful converter: the in-memory mock exists only with `SCOPEWEAVE_DEV=1`. ## Comments (코멘트) diff --git a/docs/deploy.md b/docs/deploy.md index cd08ca9a..690ee4f3 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -41,6 +41,7 @@ persists the database in the `scopeweave-data` volume. | `ORCHESTRATOR_TOKEN` | with URL | Required bearer token for the configured contextual-orchestrator service (`CONTEXTUAL_ORCHESTRATOR_TOKEN`). | | `CLEARFOLIO_URL` | for production 산출물 viewer | Root Clearfolio service origin. Production requires HTTPS and rejects credentials, paths, query strings, and fragments. When absent in production, document conversion/viewing is unavailable rather than simulated. | | `CLEARFOLIO_HMAC_SECRET` | with URL | Required tenant-claim HMAC secret; must contain at least 32 non-whitespace characters and match Clearfolio's configured verifier secret. | +| `CLEARFOLIO_ARTIFACT_ORIGINS` | optional with URL | Comma-separated exact HTTPS artifact origins (scheme, host, optional non-default port). Leave unset to trust only `CLEARFOLIO_URL`. Empty, whitespace-only, HTTP, credentialed, path, query, fragment, or non-canonical entries fail closed before any provider call. | | `SCOPEWEAVE_ATTACHMENT_STATUS_CONCURRENCY` | no (default 8, maximum 32) | Maximum concurrent Clearfolio status lookups during one attachment-list request. Invalid values fall back to 8; values above 32 are clamped. | | `SCOPEWEAVE_ATTACHMENT_STATUS_TIMEOUT_MS` | no (default 3000, maximum 30000) | Hard caller-side timeout for each Clearfolio status lookup. The AbortSignal is also forwarded downstream. | | `SCOPEWEAVE_ATTACHMENT_STATUS_BUDGET_MS` | no (default 5000, maximum 60000) | Wall-clock budget for the entire best-effort refresh pass. Work not started before the deadline is deferred to a later list request. | @@ -61,6 +62,14 @@ Keep credentials in the dedicated HMAC secret setting rather than URL userinfo, and do not configure a path, query string, or fragment. The adapter constructs its own versioned API paths from the validated origin. +Artifact links returned by Clearfolio are untrusted until they match the +provider origin or an origin listed in `CLEARFOLIO_ARTIFACT_ORIGINS`. If +Clearfolio serves files from a reviewed CDN or object store, add only that +origin, for example `https://artifacts.example.com`. Do not put signed paths, +object keys, or tokens in the setting. Unset keeps the least-privilege default: +only the Clearfolio origin is trusted, and a cross-origin `artifactToken` is +never copied into the viewer URL. + Every hosted Clearfolio request is non-redirecting and has a hard 15-second adapter budget; attachment status lookups compose that budget with the caller's own cancellation signal. Successful provider responses must be diff --git a/docs/doctoring/clearfolio-artifact-origin-trust.md b/docs/doctoring/clearfolio-artifact-origin-trust.md index 38507d3d..a48bf8cc 100644 --- a/docs/doctoring/clearfolio-artifact-origin-trust.md +++ b/docs/doctoring/clearfolio-artifact-origin-trust.md @@ -15,9 +15,9 @@ The adapter uses a positive allowlist rather than accepting any syntactically va ## Runtime contract 1. `CLEARFOLIO_ARTIFACT_ORIGINS` is optional. When absent, only the validated Clearfolio provider origin is trusted. -2. When present, the value is a comma-separated list of canonicalizable HTTPS origins. Whitespace around entries is ignored; empty entries are rejected. -3. Any malformed entry, HTTP entry, URL credential, path, query, or fragment produces `ClearfolioConfigurationError` with stable code `clearfolio_artifact_origins_invalid` before the artifact-link provider request is sent. -4. Provider-returned artifact URLs must still satisfy the existing HTTP/HTTPS and downgrade rules, must contain no credentials or fragment, and must resolve to the provider origin or an explicitly configured artifact origin. +2. When present, the value is a comma-separated list of canonical HTTPS origins. Whitespace around entries is ignored. Empty values, whitespace-only values, and empty comma-separated entries are rejected. After parsing, each trimmed entry must equal `URL.origin` or that origin plus a single trailing `/`, so default-port `:443`, empty fragments, empty userinfo, and path-normalized lookalikes cannot sneak in. +3. Any malformed entry, HTTP entry, URL credential, path, query, fragment, or non-canonical origin produces `ClearfolioConfigurationError` with stable code `clearfolio_artifact_origins_invalid` before the artifact-link provider request is sent. +4. Provider-returned artifact URLs must still satisfy the existing HTTP/HTTPS and downgrade rules, must contain no credentials or fragment, and must resolve to the provider origin or an explicitly configured artifact origin. Protocol-relative links, backslash-normalized protocol-relative paths, and userinfo-as-host URLs inherit a foreign origin and are rejected unless that origin is explicitly allowlisted. 5. A same-origin `artifactToken` may be translated into the trusted Clearfolio viewer route. A token on an approved cross-origin artifact URL remains on that returned URL; ScopeWeave never transplants it into the provider-origin viewer. 6. Exact origin comparison includes the effective port. Approving `https://cdn.example:8443` does not approve `https://cdn.example`. @@ -36,11 +36,12 @@ Do not place signed paths, object keys, tokens, credentials, query strings, or f `tests/unit/clearfolio-artifact-origin.test.mjs` exercises the production adapter with real `URL` parsing and a bounded mocked provider response. It proves: - cross-origin HTTPS artifacts fail by default; +- protocol-relative, backslash-normalized, and userinfo-as-host links do not inherit the provider origin; - same-origin relative artifacts continue to resolve against the provider; - an explicitly approved origin succeeds only for the same scheme/host/port identity; - approved cross-origin `artifactToken` values remain on the approved origin; - credentials and fragments are rejected on provider and approved origins; -- malformed, HTTP, credentialed, path-, query-, fragment-bearing, and empty-entry configuration fails before any provider transport. +- malformed, HTTP, credentialed, path-, query-, fragment-bearing, non-canonical, empty, and whitespace-only configuration fails before any provider transport. The test is registered in both the normal unit suite and the production coverage cases so changes to this boundary cannot silently bypass repository coverage evidence. diff --git a/docs/doctoring/clearfolio-production-configuration.md b/docs/doctoring/clearfolio-production-configuration.md index 2f1b1501..a4b79ca5 100644 --- a/docs/doctoring/clearfolio-production-configuration.md +++ b/docs/doctoring/clearfolio-production-configuration.md @@ -10,9 +10,9 @@ This boundary prevents configuration text from becoming an arbitrary downstream ## Artifact-token origin rule -If Clearfolio returns an `artifactToken`, ScopeWeave rewrites it into the trusted Clearfolio viewer route only when the returned URL has the same origin as the configured Clearfolio service. A token-bearing link from another origin is rejected rather than transplanted into the trusted viewer or returned directly to an unreviewed host. This closes the token-confusion boundary without claiming that arbitrary cross-origin artifact hosts are approved. +If Clearfolio returns an `artifactToken`, ScopeWeave rewrites it into the trusted Clearfolio viewer route only when the returned URL has the same origin as the configured Clearfolio service. A token-bearing link from another origin is never transplanted into the trusted viewer. Reviewed CDN or object-storage hosts are added only through the later `CLEARFOLIO_ARTIFACT_ORIGINS` allowlist recorded in `docs/doctoring/clearfolio-artifact-origin-trust.md`; without that setting, a cross-origin token remains rejected. -Issue #489 remains open after this slice. A subsequent bounded change must still implement the explicit reviewed artifact-origin allowlist, redirect policy, streaming response-size/media-type limits, provider-wide request budget, and the remaining resource/lifecycle acceptance criteria before the Clearfolio adapter can be described as fully production-complete. +Issue #489 remains open after this slice. Streaming response-size/media-type limits and the provider-wide request budget are owned by the provider-response-boundary record. Remaining work is capability readiness, persistence/lifecycle controls, incident and recovery evidence, and destination DNS/IP authorization so an allowlisted origin cannot become an arbitrary in-origin redirector. ## Executable evidence diff --git a/docs/doctoring/clearfolio-provider-response-boundary.md b/docs/doctoring/clearfolio-provider-response-boundary.md index 52a2e97a..58f8ed18 100644 --- a/docs/doctoring/clearfolio-provider-response-boundary.md +++ b/docs/doctoring/clearfolio-provider-response-boundary.md @@ -41,7 +41,7 @@ The 10 MiB limit matches the current ScopeWeave attachment API ceiling, so the d The preceding slice already prevents a cross-origin `artifactToken` from being transplanted into the trusted Clearfolio viewer origin. This slice bounds and media-validates the artifact-link response itself and disables redirects on the request. -It **does not yet approve arbitrary cross-origin artifact URLs**. Issue #489 still owns the reviewed artifact-origin allowlist and the remaining URL rules for returned links, including credential and fragment rejection. Until that later slice integrates, cross-origin artifact URLs retain the narrower predecessor behavior and must not be represented as a fully qualified production CDN/object-storage policy. +Reviewed cross-origin artifact hosts are not implied here. They are owned by the later `CLEARFOLIO_ARTIFACT_ORIGINS` allowlist in `docs/doctoring/clearfolio-artifact-origin-trust.md`, which also rejects credentials and fragments on returned links. Until that allowlist is configured, only the provider origin is trusted. Issue #489 still owns destination DNS/IP authorization, signed-artifact URL shape, and the remaining lifecycle/acceptance work. ## Verification contract diff --git a/server/clearfolio.mjs b/server/clearfolio.mjs index b4e28dd6..f7e3b5cb 100644 --- a/server/clearfolio.mjs +++ b/server/clearfolio.mjs @@ -148,9 +148,10 @@ function clearfolioArtifactOrigins(baseUrl) { } for (const entry of entries) { + const canonical = entry.trim(); let url; try { - url = new URL(entry.trim()); + url = new URL(canonical); } catch { throw new ClearfolioConfigurationError( 'clearfolio_artifact_origins_invalid', @@ -163,6 +164,7 @@ function clearfolioArtifactOrigins(baseUrl) { || url.pathname !== '/' || Boolean(url.search) || Boolean(url.hash) + || (canonical !== url.origin && canonical !== `${url.origin}/`) ) { throw new ClearfolioConfigurationError( 'clearfolio_artifact_origins_invalid', diff --git a/tests/unit/clearfolio-artifact-origin.test.mjs b/tests/unit/clearfolio-artifact-origin.test.mjs index 2094c39f..3dcf7559 100644 --- a/tests/unit/clearfolio-artifact-origin.test.mjs +++ b/tests/unit/clearfolio-artifact-origin.test.mjs @@ -44,6 +44,22 @@ test('artifact URLs default to the configured Clearfolio origin', async () => { () => resolveArtifact('https://cdn.example/file.pdf', undefined), /clearfolio artifact-link response invalid/, ); + await assert.rejects( + () => resolveArtifact('https://evil.com/file.pdf?artifactToken=stolen', undefined), + /clearfolio artifact-link response invalid/, + 'an unallowlisted token stays rejected instead of moving into the viewer', + ); + for (const link of [ + '//evil.com/file.pdf', + '/\\evil.com/file.pdf', + 'https://clearfolio.example@evil.com/x?artifactToken=t', + ]) { + await assert.rejects( + () => resolveArtifact(link, undefined), + /clearfolio artifact-link response invalid/, + `${link} must not inherit the provider origin`, + ); + } assert.equal( await resolveArtifact('/signed/file.pdf', undefined), 'https://clearfolio.example/signed/file.pdf', @@ -55,6 +71,11 @@ test('explicit HTTPS artifact origins are exact scheme-host-port allowlist entri await resolveArtifact('https://cdn.example/file.pdf', ' https://cdn.example '), 'https://cdn.example/file.pdf', ); + assert.equal( + await resolveArtifact('https://cdn.example/file.pdf', 'https://cdn.example/'), + 'https://cdn.example/file.pdf', + 'a trailing slash is still an origin-only allowlist entry', + ); assert.equal( await resolveArtifact( 'https://cdn.example/file.pdf?artifactToken=remote%20token', @@ -91,8 +112,14 @@ test('artifact origin configuration fails closed before provider transport', asy 'https://cdn.example/path', 'https://cdn.example?query=1', 'https://cdn.example#fragment', + 'https://cdn.example#', + 'https://:@cdn.example', + 'https://cdn.example/foo/..', + 'https://cdn.example:443', 'not a URL', 'https://cdn.example,', + '', + ' ', ]; for (const artifactOrigins of invalidConfigurations) { From cac0efc62c8b3f5baf1e482c8e80c96eb9c42736 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 23:36:45 +0900 Subject: [PATCH 07/12] test(clearfolio): preserve parent default-deny artifact cases --- tests/unit/clearfolio-status-signal.test.mjs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/clearfolio-status-signal.test.mjs b/tests/unit/clearfolio-status-signal.test.mjs index 70c54199..9134bb13 100644 --- a/tests/unit/clearfolio-status-signal.test.mjs +++ b/tests/unit/clearfolio-status-signal.test.mjs @@ -219,6 +219,10 @@ test('artifactUrl validates links and never exposes transport or response text', { label: 'malformed URL', json: async () => ({ artifactUrl: 'http://[' }) }, { label: 'unsupported URL scheme', json: async () => ({ artifactUrl: 'javascript:alert(1)' }) }, { label: 'HTTPS downgrade', json: async () => ({ artifactUrl: 'http://cdn.example/file.pdf' }) }, + { label: 'foreign HTTPS origin', json: async () => ({ artifactUrl: 'https://cdn.example/file.pdf' }) }, + { label: 'protocol-relative foreign origin', json: async () => ({ artifactUrl: '//evil.example/file.pdf' }) }, + { label: 'credentialed same origin', json: async () => ({ artifactUrl: 'https://user@clearfolio.example/file.pdf' }) }, + { label: 'fragmented same origin', json: async () => ({ artifactUrl: 'https://clearfolio.example/file.pdf#viewer-state' }) }, ]; for (const malformed of malformedPayloads) { @@ -245,6 +249,16 @@ test('artifactUrl validates links and never exposes transport or response text', 'same-origin artifact tokens may be translated into the trusted viewer route', ); + setResponse({ + json: async () => ({ + signedUrl: 'https://cdn.example/file.pdf?artifactToken=token%20value', + }), + }); + await expectSanitizedFailure( + () => artifactUrl(4, 5, 'job-1'), + 'clearfolio artifact-link response invalid', + ); + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = 'https://cdn.example'; try { setResponse({ json: async () => ({ url: 'https://cdn.example/file.pdf' }) }); From 243fe1299d255e2f0be89fb23e6410f206734c74 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 23:37:29 +0900 Subject: [PATCH 08/12] docs(clearfolio): preserve parent redirect and default-deny contract --- .../clearfolio-production-configuration.md | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/doctoring/clearfolio-production-configuration.md b/docs/doctoring/clearfolio-production-configuration.md index a4b79ca5..8c727230 100644 --- a/docs/doctoring/clearfolio-production-configuration.md +++ b/docs/doctoring/clearfolio-production-configuration.md @@ -8,11 +8,15 @@ A configured production provider must be a root HTTPS origin. ScopeWeave parses This boundary prevents configuration text from becoming an arbitrary downstream request prefix and prevents a production deployment from persisting fake `SUCCEEDED` conversion state merely because an integration is absent. It also preserves independent ScopeWeave operation: planning functionality remains available while document conversion/viewing fails closed with an actionable configuration error. -## Artifact-token origin rule +## Provider redirect and artifact-origin rule -If Clearfolio returns an `artifactToken`, ScopeWeave rewrites it into the trusted Clearfolio viewer route only when the returned URL has the same origin as the configured Clearfolio service. A token-bearing link from another origin is never transplanted into the trusted viewer. Reviewed CDN or object-storage hosts are added only through the later `CLEARFOLIO_ARTIFACT_ORIGINS` allowlist recorded in `docs/doctoring/clearfolio-artifact-origin-trust.md`; without that setting, a cross-origin token remains rejected. +Every tenant-signed submit, status, and artifact-link fetch uses `redirect: "error"`. A provider redirect therefore becomes the existing sanitized transport failure instead of allowing the runtime to replay tenant HMAC headers onto an untrusted `Location` target. -Issue #489 remains open after this slice. Streaming response-size/media-type limits and the provider-wide request budget are owned by the provider-response-boundary record. Remaining work is capability readiness, persistence/lifecycle controls, incident and recovery evidence, and destination DNS/IP authorization so an allowlisted origin cannot become an arbitrary in-origin redirector. +Artifact links are default-deny outside the configured Clearfolio origin. They must contain no URL credentials or fragment. Protocol-relative or absolute foreign-host links therefore fail closed unless the foreign origin has been explicitly admitted by the reviewed `CLEARFOLIO_ARTIFACT_ORIGINS` policy described in `docs/doctoring/clearfolio-artifact-origin-trust.md`. Allowlist entries themselves must be canonical origin values so ambiguous path, credential, fragment, and serialization variants cannot silently broaden authority. + +If a same-origin link contains an `artifactToken`, ScopeWeave rewrites that token into the trusted Clearfolio viewer route. A token is never transplanted from one origin into another. When a reviewed cross-origin artifact origin is explicitly allowed, its token-bearing URL remains bound to that issuing origin rather than being rewritten into the Clearfolio viewer. + +Issue #489 remains open after this stack. The provider-response parent owns bounded request time, response media type/size, streamed JSON parsing, and response-body cleanup. Remaining work includes capability readiness, broader persistence/lifecycle and incident/recovery evidence, plus destination DNS/IP authorization and any stronger signed-artifact URL-shape restrictions required for allowlisted origins. ## Executable evidence @@ -24,21 +28,21 @@ Issue #489 remains open after this slice. Streaming response-size/media-type lim - loopback HTTP is accepted only under explicit development mode; and - signed tenant headers retain the documented canonical HMAC contract. -`tests/unit/clearfolio-status-signal.test.mjs` continues to exercise sanitized transport/HTTP/JSON/status/artifact failures and now proves that a cross-origin token-bearing artifact link fails closed rather than moving the token into the Clearfolio viewer or returning it to an unreviewed host. `tests/api/attachment-status.test.mjs` makes its test-only in-memory provider explicit instead of relying on an unset production URL. +`tests/unit/clearfolio-status-signal.test.mjs` preserves the parent transport/HTTP/JSON/status/artifact regressions and requires all three tenant-signed fetch paths to disable redirects. With no artifact-origin allowlist it rejects token-free foreign HTTPS links, protocol-relative foreign links, credential-bearing same-origin links, fragmented same-origin links, and cross-origin token-bearing links while retaining same-origin relative links and the trusted viewer rewrite. With an explicit canonical `CLEARFOLIO_ARTIFACT_ORIGINS` entry it permits that exact foreign origin and leaves a foreign-origin token bound to its issuing URL. `tests/api/attachment-status.test.mjs` keeps its test-only in-memory provider explicit instead of relying on an unset production URL. -The shipped `server/clearfolio.mjs` remains in the canonical c8 production coverage target, so the new configuration branches execute under the repository coverage gate rather than a documentation-only path. +The shipped `server/clearfolio.mjs` remains in the canonical c8 production coverage target, so the configuration, provider-response, and allowlist branches execute under the repository coverage gate rather than a documentation-only path. ## Standards and threat rationale -The WHATWG URL Standard defines URL components, including credentials, queries, and fragments, and provides the common parsing model used by the JavaScript `URL` API. ScopeWeave parses first and then applies component-level policy instead of relying on string-prefix validation. +The WHATWG URL Standard defines URL components, credentials, origins, queries, fragments, and serialization behavior used by the JavaScript `URL` API. ScopeWeave parses first and then applies component-level and canonical-origin policy instead of relying on string-prefix validation. -OWASP's SSRF Prevention guidance recommends strict allowlisting and warns that redirects and attacker-controlled complete URLs can bypass URL validation. This slice narrows operator configuration to a provider origin and keeps request paths adapter-owned. The remaining redirect and artifact-host controls stay explicitly tracked by issue #489 rather than being implied by this narrower change. +OWASP's SSRF Prevention guidance recommends strict allowlisting and warns that redirects and attacker-controlled complete URLs can bypass URL validation. ScopeWeave narrows operator configuration to a provider origin, disables redirect following for tenant-signed calls, defaults artifact redirects to the provider origin, and admits a foreign artifact origin only through explicit canonical allowlisting. DNS/IP destination authorization remains a separately tracked defense-in-depth boundary rather than being implied by hostname allowlisting. NIST SSDF 1.1 recommends identifying and maintaining software security requirements and producing well-secured software through repeatable verification. The fail-closed configuration contract, executable negative tests, and explicit remaining-gap statement provide acquisition-review evidence without claiming certification. ## Rollback -Rollback reverts the Clearfolio configuration parser, explicit development-mode tests, token-origin rule, deployment text, this doctoring record, and the corresponding CHANGELOG entry together. No database schema or persisted attachment representation changes in this slice. +Rolling back the artifact-origin child removes its allowlist parsing, allowlist-specific tests/docs/deployment text, and child CHANGELOG entries while retaining the parent provider-configuration, redirect prohibition, same-origin default, bounded-response, and request-budget controls. No database schema or persisted attachment representation changes in this slice. ## References From ae5daff7c48b0430989cc238a999067902c4aa73 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 26 Aug 2026 05:02:08 -0700 Subject: [PATCH 09/12] test(clearfolio): retain parent timeout-disposal contract --- .../clearfolio-provider-boundary.test.mjs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/unit/clearfolio-provider-boundary.test.mjs b/tests/unit/clearfolio-provider-boundary.test.mjs index 6797d53d..0f153815 100644 --- a/tests/unit/clearfolio-provider-boundary.test.mjs +++ b/tests/unit/clearfolio-provider-boundary.test.mjs @@ -54,6 +54,35 @@ test('provider requests disable redirects and carry a bounded total-request sign assert.equal(CLEARFOLIO_REQUEST_TIMEOUT_MS > 0 && CLEARFOLIO_REQUEST_TIMEOUT_MS <= 30_000, true); }); +test('completed provider requests dispose their timeout timers immediately', async () => { + useResponse({ status: 'RUNNING' }); + const originalSetTimeout = globalThis.setTimeout; + const originalClearTimeout = globalThis.clearTimeout; + const timer = { unref() {} }; + let scheduled = 0; + let cleared = 0; + + globalThis.setTimeout = (callback, delay) => { + assert.equal(typeof callback, 'function'); + assert.equal(delay, CLEARFOLIO_REQUEST_TIMEOUT_MS); + scheduled += 1; + return timer; + }; + globalThis.clearTimeout = (value) => { + assert.equal(value, timer); + cleared += 1; + }; + + try { + assert.equal(await jobStatus(1, 2, 'job-1'), 'RUNNING'); + assert.equal(scheduled, 1, 'one bounded provider timer is created'); + assert.equal(cleared, 1, 'the completed request clears its provider timer'); + } finally { + globalThis.setTimeout = originalSetTimeout; + globalThis.clearTimeout = originalClearTimeout; + } +}); + test('status response requires JSON media type before parsing', async () => { useResponse({ status: 'RUNNING' }, { contentType: 'text/plain' }); await assert.rejects( From 6b4785760856425227fd3c874e6bcf4684a60549 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 26 Aug 2026 05:05:42 -0700 Subject: [PATCH 10/12] fix(clearfolio): reconcile provider timer disposal --- server/clearfolio.mjs | 209 ++++++++++++++++++++++++++---------------- 1 file changed, 128 insertions(+), 81 deletions(-) diff --git a/server/clearfolio.mjs b/server/clearfolio.mjs index f7e3b5cb..76772fff 100644 --- a/server/clearfolio.mjs +++ b/server/clearfolio.mjs @@ -299,16 +299,43 @@ function validateJobId(jobId) { } /** - * Compose an optional caller cancellation signal with the hard provider budget. + * Compose caller cancellation with a hard provider budget that can be disposed. + * + * The returned scope stays active until the provider response body has been + * fully validated or cancelled. Callers must dispose it in `finally` so fast + * requests do not retain a timeout or caller-signal listener for the full budget. * * @param {AbortSignal|undefined} callerSignal - Optional upstream cancellation signal. - * @returns {AbortSignal} Signal that aborts on caller cancellation or total timeout. + * @returns {{signal:AbortSignal,dispose:()=>void}} Scoped provider cancellation contract. */ function providerSignal(callerSignal) { - const timeoutSignal = AbortSignal.timeout(CLEARFOLIO_REQUEST_TIMEOUT_MS); - if (callerSignal === undefined) return timeoutSignal; - if (!(callerSignal instanceof AbortSignal)) throw new TypeError('signal must be an AbortSignal'); - return AbortSignal.any([callerSignal, timeoutSignal]); + if (callerSignal !== undefined && !(callerSignal instanceof AbortSignal)) { + throw new TypeError('signal must be an AbortSignal'); + } + + const controller = new AbortController(); + const timeoutError = new DOMException('Clearfolio provider request timed out', 'TimeoutError'); + const timeoutId = setTimeout( + controller.abort.bind(controller, timeoutError), + CLEARFOLIO_REQUEST_TIMEOUT_MS, + ); + timeoutId.unref(); + + const abortFromCaller = controller.abort.bind(controller); + if (callerSignal !== undefined) { + if (callerSignal.aborted) controller.abort(callerSignal.reason); + else callerSignal.addEventListener('abort', abortFromCaller, { once: true }); + } + + return { + signal: controller.signal, + dispose() { + clearTimeout(timeoutId); + if (callerSignal !== undefined) { + callerSignal.removeEventListener('abort', abortFromCaller); + } + }, + }; } /** @@ -450,32 +477,37 @@ export async function submitJob(orgId, userId, document) { new Blob([validatedDocument.bytes], { type: validatedDocument.mime || 'application/octet-stream' }), validatedDocument.name, ); - let res; - try { - res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs`, { - method: 'POST', - headers: tenantHeaders(orgId, userId, configuration.secret), - body: form, - redirect: 'error', - signal: providerSignal(), - }); - } catch { - throw new Error('clearfolio submit unavailable'); - } - if (!res.ok) return rejectProviderResponse(res, `clearfolio submit failed (${res.status})`); - const data = await readBoundedJson(res, 'clearfolio submit response invalid'); - if (!isJsonRecord(data)) throw new Error('clearfolio submit response invalid'); - const status = data.status === undefined ? 'PENDING' : data.status; - if (typeof data.jobId !== 'string' || !isClearfolioJobStatus(status)) { - throw new Error('clearfolio submit response invalid'); - } - let jobId; + const request = providerSignal(); try { - jobId = validateJobId(data.jobId); - } catch { - throw new Error('clearfolio submit response invalid'); + let res; + try { + res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs`, { + method: 'POST', + headers: tenantHeaders(orgId, userId, configuration.secret), + body: form, + redirect: 'error', + signal: request.signal, + }); + } catch { + throw new Error('clearfolio submit unavailable'); + } + if (!res.ok) return rejectProviderResponse(res, `clearfolio submit failed (${res.status})`); + const data = await readBoundedJson(res, 'clearfolio submit response invalid'); + if (!isJsonRecord(data)) throw new Error('clearfolio submit response invalid'); + const status = data.status === undefined ? 'PENDING' : data.status; + if (typeof data.jobId !== 'string' || !isClearfolioJobStatus(status)) { + throw new Error('clearfolio submit response invalid'); + } + let jobId; + try { + jobId = validateJobId(data.jobId); + } catch { + throw new Error('clearfolio submit response invalid'); + } + return { jobId, status }; + } finally { + request.dispose(); } - return { jobId, status }; } /** @@ -497,22 +529,32 @@ export async function jobStatus(orgId, userId, jobId, { signal } = {}) { const canonicalJobId = validateJobId(jobId); const configuration = clearfolioConfiguration(); if (configuration.mock) return mockDocs.has(canonicalJobId) ? 'SUCCEEDED' : 'FAILED'; - let res; + let request; try { - res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs/${encodeURIComponent(canonicalJobId)}`, { - headers: tenantHeaders(orgId, userId, configuration.secret), - signal: providerSignal(signal), - redirect: 'error', - }); + request = providerSignal(signal); } catch { throw new Error('clearfolio status unavailable'); } - if (!res.ok) return rejectProviderResponse(res, `clearfolio status failed (${res.status})`); - const data = await readBoundedJson(res, 'clearfolio status response invalid'); - if (!isJsonRecord(data) || !isClearfolioJobStatus(data.status)) { - throw new Error('clearfolio status response invalid'); + try { + let res; + try { + res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs/${encodeURIComponent(canonicalJobId)}`, { + headers: tenantHeaders(orgId, userId, configuration.secret), + signal: request.signal, + redirect: 'error', + }); + } catch { + throw new Error('clearfolio status unavailable'); + } + if (!res.ok) return rejectProviderResponse(res, `clearfolio status failed (${res.status})`); + const data = await readBoundedJson(res, 'clearfolio status response invalid'); + if (!isJsonRecord(data) || !isClearfolioJobStatus(data.status)) { + throw new Error('clearfolio status response invalid'); + } + return data.status; + } finally { + request.dispose(); } - return data.status; } /** @@ -533,48 +575,53 @@ export async function artifactUrl(orgId, userId, jobId) { const configuration = clearfolioConfiguration(); if (configuration.mock) return `/api/mock-clearfolio/${encodeURIComponent(canonicalJobId)}`; const trustedArtifactOrigins = clearfolioArtifactOrigins(configuration.baseUrl); - let res; + const request = providerSignal(); try { - res = await fetch(`${configuration.baseUrl}/api/v1/viewer/${encodeURIComponent(canonicalJobId)}/artifact-links`, { - method: 'POST', - headers: tenantHeaders(orgId, userId, configuration.secret), - redirect: 'error', - signal: providerSignal(), - }); - } catch { - throw new Error('clearfolio artifact-link unavailable'); - } - if (!res.ok) return rejectProviderResponse(res, `clearfolio artifact-link failed (${res.status})`); - const data = await readBoundedJson(res, 'clearfolio artifact-link response invalid'); - if (!isJsonRecord(data)) throw new Error('clearfolio artifact-link response invalid'); - const link = data.artifactUrl || data.url || data.signedUrl; - if (typeof link !== 'string' || link.length === 0) { - throw new Error('clearfolio artifact-link response invalid'); - } + let res; + try { + res = await fetch(`${configuration.baseUrl}/api/v1/viewer/${encodeURIComponent(canonicalJobId)}/artifact-links`, { + method: 'POST', + headers: tenantHeaders(orgId, userId, configuration.secret), + redirect: 'error', + signal: request.signal, + }); + } catch { + throw new Error('clearfolio artifact-link unavailable'); + } + if (!res.ok) return rejectProviderResponse(res, `clearfolio artifact-link failed (${res.status})`); + const data = await readBoundedJson(res, 'clearfolio artifact-link response invalid'); + if (!isJsonRecord(data)) throw new Error('clearfolio artifact-link response invalid'); + const link = data.artifactUrl || data.url || data.signedUrl; + if (typeof link !== 'string' || link.length === 0) { + throw new Error('clearfolio artifact-link response invalid'); + } - let url; - let clearfolioUrl; - try { - clearfolioUrl = new URL(configuration.baseUrl); - url = new URL(link, clearfolioUrl); - } catch { - throw new Error('clearfolio artifact-link response invalid'); - } - const allowsHttp = clearfolioUrl.protocol === 'http:' && url.protocol === 'http:'; - if (url.protocol !== 'https:' && !allowsHttp) { - throw new Error('clearfolio artifact-link response invalid'); - } - if ( - Boolean(url.username + url.password) - || Boolean(url.hash) - || !trustedArtifactOrigins.has(url.origin) - ) { - throw new Error('clearfolio artifact-link response invalid'); - } + let url; + let clearfolioUrl; + try { + clearfolioUrl = new URL(configuration.baseUrl); + url = new URL(link, clearfolioUrl); + } catch { + throw new Error('clearfolio artifact-link response invalid'); + } + const allowsHttp = clearfolioUrl.protocol === 'http:' && url.protocol === 'http:'; + if (url.protocol !== 'https:' && !allowsHttp) { + throw new Error('clearfolio artifact-link response invalid'); + } + if ( + Boolean(url.username + url.password) + || Boolean(url.hash) + || !trustedArtifactOrigins.has(url.origin) + ) { + throw new Error('clearfolio artifact-link response invalid'); + } - const token = url.searchParams.get('artifactToken'); - if (token && url.origin === clearfolioUrl.origin) { - return `${configuration.baseUrl}/viewer/${encodeURIComponent(canonicalJobId)}?artifactToken=${encodeURIComponent(token)}`; + const token = url.searchParams.get('artifactToken'); + if (token && url.origin === clearfolioUrl.origin) { + return `${configuration.baseUrl}/viewer/${encodeURIComponent(canonicalJobId)}?artifactToken=${encodeURIComponent(token)}`; + } + return url.href; + } finally { + request.dispose(); } - return url.href; } From 6ad5a8f0aaf1de03b14c070fa4d5589e77cb5a4b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 26 Aug 2026 15:33:22 -0700 Subject: [PATCH 11/12] test(clearfolio): preserve provider timeout classification --- .../unit/clearfolio-refresh-timeout.test.mjs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/unit/clearfolio-refresh-timeout.test.mjs diff --git a/tests/unit/clearfolio-refresh-timeout.test.mjs b/tests/unit/clearfolio-refresh-timeout.test.mjs new file mode 100644 index 00000000..5e5eacec --- /dev/null +++ b/tests/unit/clearfolio-refresh-timeout.test.mjs @@ -0,0 +1,62 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { refreshAttachmentStatuses } from '../../server/attachment_status.mjs'; + +const HMAC_SECRET = 'clearfolio-shared-secret-32-bytes!!'; +const originalFetch = globalThis.fetch; +process.env.CLEARFOLIO_URL = 'https://clearfolio.example'; +process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; + +globalThis.fetch = async () => { + throw new DOMException('private provider timeout detail', 'TimeoutError'); +}; + +const { jobStatus } = await import( + '../../server/clearfolio.mjs?refresh-timeout-classification-test=1' +); + +test.after(() => { + globalThis.fetch = originalFetch; + delete process.env.CLEARFOLIO_URL; + delete process.env.CLEARFOLIO_HMAC_SECRET; +}); + +test('provider timeout stays sanitized and is counted as a refresh timeout', async () => { + await assert.rejects( + () => jobStatus(1, 2, 'job-timeout'), + (error) => { + assert.equal(error.name, 'TimeoutError'); + assert.equal(error.message, 'clearfolio status unavailable'); + assert.doesNotMatch(error.message, /private provider timeout detail/); + return true; + }, + ); + + const rows = [{ id: 1, jobId: 'job-timeout', status: 'PENDING' }]; + const categories = []; + const metrics = {}; + const counts = await refreshAttachmentStatuses(rows, { + orgId: 1, + userId: 2, + timeoutMs: 30_000, + budgetMs: 60_000, + metrics, + onError: ({ category }) => categories.push(category), + jobStatus, + updateStatus: () => { + throw new Error('timed-out status must not be persisted'); + }, + }); + + assert.deepEqual(counts, { + attempted: 1, + changed: 0, + failed: 1, + skipped: 0, + deferred: 0, + }); + assert.deepEqual(categories, ['timeout']); + assert.equal(metrics.attachmentStatusRefreshTimeoutFailures, 1); + assert.equal(metrics.attachmentStatusRefreshDownstreamLookupFailures, 0); + assert.equal(rows[0].status, 'PENDING'); +}); From ceaa9821da68c14da5243e456a44d140e80c2354 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 28 Aug 2026 13:13:21 -0700 Subject: [PATCH 12/12] fix(stack): preserve parent timeout classification --- server/attachment_status.mjs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/attachment_status.mjs b/server/attachment_status.mjs index c6f9ae21..3471ca3b 100644 --- a/server/attachment_status.mjs +++ b/server/attachment_status.mjs @@ -281,9 +281,10 @@ export async function refreshAttachmentStatuses(rows, options) { } } catch (error) { counts.failed += 1; - const category = error?.name === ATTACHMENT_STATUS_TIMEOUT_ERROR - ? 'timeout' - : failureCategory; + const category = ( + error?.name === ATTACHMENT_STATUS_TIMEOUT_ERROR + || (failureCategory === 'downstream_lookup' && error?.name === 'TimeoutError') + ) ? 'timeout' : failureCategory; failureCounts[category] += 1; reportRefreshFailure(options.onError, category); }