diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3006c74b..16697dd3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -25,6 +25,13 @@ app flows. - `tests/config/`: repository governance and workflow ownership checks. +## Clearfolio adapter + +- Clearfolio is an optional MSA document-viewer adapter. Planning stays available when it is unconfigured. +- Production requires a root HTTPS origin and HMAC secret; the in-memory adapter exists only behind `SCOPEWEAVE_DEV=1`. +- Hosted provider calls are non-redirecting, time-bounded, and JSON-bounded before any provider data can change ScopeWeave state. +- Attachment view 302s only to the configured origin or `CLEARFOLIO_ARTIFACT_ORIGINS`. Credentials and fragments are rejected. + ## Core decisions - One global `tasks` array holds canonical task records. diff --git a/CHANGELOG.md b/CHANGELOG.md index 922a4903..60c68da6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,14 @@ 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. +- 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 + before allocation, persistence, or URL construction. +- Restricted Clearfolio attachment-view redirects to the configured provider + origin or an explicit `CLEARFOLIO_ARTIFACT_ORIGINS` allowlist, and rejected + credential-bearing or fragment-bearing artifact links before the 302 target + is returned. - Made `SCOPEWEAVE_JWT_SECRET` mandatory at startup and rejected weak or unexpanded placeholder values so production deployments fail closed. - Neutralized audit-log CSV formulas even when executable prefixes are hidden diff --git a/README.md b/README.md index ea8bb77a..71b8068b 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,8 @@ Docker: set a **persistent** `SCOPEWEAVE_JWT_SECRET` first, then run `docker com | `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) | +| `CLEARFOLIO_URL` / `CLEARFOLIO_HMAC_SECRET` | Production document viewer origin and HMAC secret | +| `CLEARFOLIO_ARTIFACT_ORIGINS` | Reviewed CDN origins for attachment-view redirects | ## Verification diff --git a/docs/api.md b/docs/api.md index 1c668425..bbea53c5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -87,11 +87,14 @@ credentials never reach the browser. HWP/HWPX are rejected (Clearfolio policy). | --- | --- | --- | | `POST` | `/api/projects/:id/attachments` | multipart `file` (+`taskId?`, ≤10MB) → conversion job (write roles) | | `GET` | `/api/projects/:id/attachments?taskId=` | List (+ refreshes pending statuses) | -| `GET` | `/api/projects/:id/attachments/:aid/view` | 302 → signed artifact URL (`?token=` for new-tab opens) | +| `GET` | `/api/projects/:id/attachments/:aid/view` | 302 → same-origin or allowlisted 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` and `CLEARFOLIO_HMAC_SECRET` for production conversion. +`CLEARFOLIO_ARTIFACT_ORIGINS` lists reviewed CDN origins for attachment-view +redirects; empty trusts only the Clearfolio origin. Unset `CLEARFOLIO_URL` +fails closed in production and enables the in-memory converter only with +`SCOPEWEAVE_DEV=1`. ## Comments (코멘트) diff --git a/docs/deploy.md b/docs/deploy.md index 83a099b3..3fc5fd46 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` | with URL, when artifacts are served from a CDN | Comma-separated reviewed HTTPS origin list for attachment-view redirects. Empty means only the configured Clearfolio origin is trusted. Entries must be origin URLs without credentials, paths, query strings, or fragments. | | `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,28 @@ 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. +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 +`application/json`, and both declared and streamed response bytes are capped at +256 KiB before JSON parsing. Provider job identifiers are limited to 256 +characters, and upload metadata/bytes are validated before Blob/FormData +allocation. The adapter's document ceiling is 10 MiB, matching the current +ScopeWeave attachment API limit. + +These limits are code constants rather than operator-tunable environment +settings. A deployment that needs larger provider responses, longer requests, or +larger documents requires a reviewed application change with corresponding +resource, latency, and security evidence; do not work around these bounds at the +proxy layer. + +Attachment view 302s the browser to the provider-returned artifact URL. That +URL is accepted only when it has no credentials or fragment and its origin is +the configured Clearfolio origin or an origin listed in +`CLEARFOLIO_ARTIFACT_ORIGINS`. If planners cannot open a converted document +hosted on a CDN, add that reviewed origin to the allowlist or keep the file on +the Clearfolio host. + ## Attachment status refresh operations The attachment-list API reads `job_id` in its initial project-scoped query and diff --git a/docs/doctoring/clearfolio-artifact-origin-allowlist.md b/docs/doctoring/clearfolio-artifact-origin-allowlist.md new file mode 100644 index 00000000..62a17f33 --- /dev/null +++ b/docs/doctoring/clearfolio-artifact-origin-allowlist.md @@ -0,0 +1,65 @@ +# Clearfolio artifact-origin allowlist + +## Decision + +Attachment view is a browser 302 to a provider-returned artifact URL. ScopeWeave therefore treats that URL as an untrusted redirect target even after the Clearfolio origin, HMAC, transport, and JSON-response boundaries have passed. + +A returned link is accepted only when: + +1. it parses as an absolute URL against the configured Clearfolio origin; +2. its scheme is HTTPS, or HTTP only when both the provider and the link are explicit development-mode loopback HTTP; +3. it has no userinfo and no fragment; +4. its origin is either the configured Clearfolio origin or an origin listed in `CLEARFOLIO_ARTIFACT_ORIGINS`. + +Same-origin `artifactToken` values may still be rewritten into the trusted Clearfolio viewer route. Tokens on an allowlisted CDN stay on that CDN and are never copied into the viewer host. An empty allowlist means only the configured Clearfolio origin is trusted, so a planner cannot be sent to an unreviewed host by a confused or compromised provider response. + +This slice does not invent a second wire protocol. It only constrains which returned URLs ScopeWeave will hand to the browser. Retry, idempotency, and the remaining Clearfolio lifecycle work stay on issue #489. + +```mermaid +flowchart TD + view["GET attachment view"] --> provider["Clearfolio artifact-link JSON"] + provider --> parse["WHATWG URL parse"] + parse --> scheme{"HTTPS or explicit loopback HTTP?"} + scheme -->|no| reject["Fail closed: invalid artifact link"] + scheme -->|yes| secrets{"Userinfo or fragment?"} + secrets -->|yes| reject + secrets -->|no| origin{"Same Clearfolio origin or listed in CLEARFOLIO_ARTIFACT_ORIGINS?"} + origin -->|no| reject + origin -->|yes| token{"Same-origin artifactToken?"} + token -->|yes| viewer["Redirect to Clearfolio viewer"] + token -->|no| host["Redirect to that origin; tokens stay put"] +``` + +## Operator action + +To serve converted documents from a reviewed CDN or object-storage origin, set `CLEARFOLIO_ARTIFACT_ORIGINS` to a comma-separated list of absolute origin URLs such as `https://cdn.example,https://files.example:8443`. Leave the setting empty to keep viewing on the Clearfolio origin only. If a planner sees `clearfolio artifact-link response invalid` after a successful conversion, add the reviewed origin or keep the file on the Clearfolio host; do not disable the check at a proxy. + +Unsafe allowlist entries fail closed with `clearfolio_artifact_origins_invalid` and tell the operator to correct the origin list. Credentials, paths, query strings, fragments, and remote HTTP are rejected in the allowlist itself so the setting cannot become an arbitrary request prefix. + +## Verification contract + +`tests/unit/clearfolio-artifact-origin.test.mjs` proves: + +- tokenless cross-origin links fail without an allowlist; +- userinfo and fragments never become redirect targets, even when the host is listed; +- same-origin relative and token-bearing viewer links still work with an empty allowlist; +- listed CDN origins may be returned, and their tokens stay on that origin; +- unlisted CDN origins and malformed allowlist entries fail closed. + +`tests/unit/clearfolio-status-signal.test.mjs` now expects the predecessor `https://cdn.example/file.pdf` fixture to fail closed unless that origin is reviewed. The adapter remains an owned c8 production target. + +## Security rationale + +OWASP's SSRF guidance treats attacker-controlled complete URLs and open redirects as bypass paths around host validation. Attachment view is a user-facing redirect, so the same allowlist discipline applies before the 302 is issued. The WHATWG URL Standard supplies the component model (origin, userinfo, fragment) used for exact comparison instead of string-prefix checks. OWASP API10:2023 continues to classify unvalidated third-party data as unsafe API consumption. + +## Rollback + +Rollback reverts the allowlist parser, artifact-link origin/credential/fragment checks, the new and adapted unit regressions, test registrations, deployment and API guidance, this doctoring record, and the CHANGELOG entry together. No database schema or persisted attachment representation changes in this slice. + +## References + +OWASP Foundation. (2023). *API10:2023 unsafe consumption of APIs*. OWASP API Security Top 10. https://owasp.org/API-Security/editions/2023/en/0xaa-unsafe-consumption-of-apis/ + +OWASP Foundation. (n.d.). *Server Side Request Forgery Prevention Cheat Sheet*. OWASP Cheat Sheet Series. https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html + +WHATWG. (2026). *URL Standard*. https://url.spec.whatwg.org/ diff --git a/docs/doctoring/clearfolio-production-configuration.md b/docs/doctoring/clearfolio-production-configuration.md index 2f1b1501..0a72336d 100644 --- a/docs/doctoring/clearfolio-production-configuration.md +++ b/docs/doctoring/clearfolio-production-configuration.md @@ -12,7 +12,7 @@ This boundary prevents configuration text from becoming an arbitrary downstream 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. -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. Transport, JSON-response, and artifact-origin allowlist rules now live in their successor doctoring records. Remaining #489 work is retry/idempotency, capability readiness, persistence/lifecycle, and protected integration—not another configuration parser. ## Executable evidence diff --git a/docs/doctoring/clearfolio-provider-response-boundary.md b/docs/doctoring/clearfolio-provider-response-boundary.md new file mode 100644 index 00000000..0879862b --- /dev/null +++ b/docs/doctoring/clearfolio-provider-response-boundary.md @@ -0,0 +1,82 @@ +# Clearfolio provider response and request boundary + +## Decision + +ScopeWeave treats the Clearfolio service as an untrusted external API even after its root origin and tenant HMAC configuration have passed the production configuration boundary. Every hosted submit, status, and artifact-link call therefore uses the same fail-closed transport and response rules before provider data can affect ScopeWeave state or browser-visible behavior. + +This record is intentionally narrower than the full Clearfolio production-readiness issue. It extends the configuration boundary introduced by the preceding production-configuration slice and does not claim that arbitrary cross-origin artifact delivery, retry/idempotency policy, or the complete provider lifecycle is finished. + +## Request contract + +Hosted provider requests: + +1. use the configuration-validated provider origin and adapter-owned endpoint path; +2. send tenant claims only to that direct origin request; +3. use Fetch `redirect: "error"` so a redirect is a transport failure rather than a credential-forwarding opportunity; +4. carry a hard 15,000 ms total-request `AbortSignal`; +5. compose a caller cancellation signal with that hard budget for status refreshes; and +6. collapse network, redirect, timeout, and cancellation details into fixed operation-level errors before they can reach browser or diagnostic payloads. + +ScopeWeave does not retry provider calls in this slice. Retry eligibility, idempotency keys, backoff, cancellation recovery, and persisted operation lifecycle remain explicit follow-up work rather than being guessed at the transport layer. + +## Response contract + +Successful provider responses are accepted only when the media type essence is `application/json`. If `Content-Length` is present it must be an exact non-negative decimal integer no greater than 256 KiB. The body stream is independently counted to the same 256 KiB ceiling, so missing or dishonest length metadata cannot bypass the resource limit. Empty bodies, malformed streams, invalid UTF-8, malformed JSON, and incompatible JSON shapes fail closed with fixed operation-specific errors. + +The adapter never uses `response.json()` directly for successful hosted provider responses. This prevents an otherwise successful response from being buffered without an application-level byte ceiling before validation. + +Provider conversion states remain the exact `PENDING`, `RUNNING`, `SUCCEEDED`, and `FAILED` set. Provider job identifiers are trimmed and limited to 256 characters without control characters before persistence or URL construction. + +## Document boundary + +Document metadata and bytes are validated before `Blob` or `FormData` construction. The provider adapter accepts only: + +- a non-empty document name of at most 512 characters without control characters; +- a MIME string of at most 255 characters without control characters; an empty value retains the existing `application/octet-stream` fallback; +- `Uint8Array`-compatible bytes no larger than 10 MiB. + +The 10 MiB limit matches the current ScopeWeave attachment API ceiling, so the downstream adapter cannot accept a document larger than the application path that feeds it. + +## Artifact boundary and remaining work + +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. + +The successor artifact-origin slice owns credential, fragment, and reviewed-origin allowlist rules for attachment-view redirects. This transport slice must not be described as approving arbitrary cross-origin artifact URLs. Issue #489 still owns retry/idempotency, capability readiness, and the remaining provider lifecycle. + +## Verification contract + +Regression evidence covers: + +- `redirect: "error"` on submit, status, and artifact-link calls; +- hard request-budget signals and caller-signal composition; +- non-JSON media rejection; +- declared and streamed response-size overflow; +- cancellation-detail sanitization; +- document metadata/byte rejection before provider transport; +- oversized provider job identifiers before URL construction; +- valid streamed JSON compatibility for submission, status, HMAC, loopback-development, and artifact-link behavior; +- the predecessor configuration, HMAC, artifact-token-origin, sanitized-error, status-enum, and attachment-refresh contracts under the same normal unit/coverage paths. + +`server/clearfolio.mjs` remains an owned c8 production target. The new provider-boundary regression is registered in both `test:unit` and `test:coverage:cases`; exact statement, branch, function, and line evidence remains a merge gate rather than a documentation claim. + +## Security rationale + +OWASP API10:2023 identifies unsafe consumption of third-party APIs when applications trust integrated-service data, blindly follow redirects, fail to validate returned data, omit timeouts, or fail to limit resources used to process third-party responses. OWASP API4:2023 separately highlights unbounded memory, bandwidth, and execution-time consumption. The transport, timeout, media-type, streaming-byte, identifier, and document limits in this slice apply those controls at the provider boundary rather than relying on Clearfolio to behave correctly. + +The WHATWG Fetch Standard explicitly supports `redirect: "error"` to reject redirect responses. ScopeWeave uses that mode because tenant HMAC claims are provider-origin credentials and there is no reviewed redirect allowlist in the current protocol. + +Node.js 22 provides `AbortSignal.timeout()` and `AbortSignal.any()`, allowing the adapter to impose its own total request budget while preserving upstream cancellation without maintaining a second timer/cancellation protocol. + +## Rollback + +Rollback reverts the provider-boundary implementation, the new and adapted unit regressions, test registrations, deployment guidance, this doctoring record, and the CHANGELOG entry together. The slice adds no database schema or migration. Existing persisted attachment state remains readable by the predecessor implementation. + +## References + +Node.js contributors. (2026). *Global objects: AbortSignal*. Node.js documentation. Retrieved August 15, 2026, from https://nodejs.org/download/release/v22.18.0/docs/api/globals.html + +OWASP Foundation. (2023a). *API4:2023 unrestricted resource consumption*. OWASP API Security Top 10. https://owasp.org/API-Security/editions/2023/en/0xa4-unrestricted-resource-consumption/ + +OWASP Foundation. (2023b). *API10:2023 unsafe consumption of APIs*. OWASP API Security Top 10. https://owasp.org/API-Security/editions/2023/en/0xaa-unsafe-consumption-of-apis/ + +WHATWG. (2026). *Fetch Standard* (Living Standard, updated May 8, 2026). https://fetch.spec.whatwg.org/ diff --git a/docs/security.md b/docs/security.md index 0ceee972..d78bc5d5 100644 --- a/docs/security.md +++ b/docs/security.md @@ -13,6 +13,10 @@ ScopeWeave treats the following controls as release-blocking invariants. A chang Bearer-token middleware and every endpoint that accepts a JWT through another transport must compare the token's `tv` claim with the user's current database `token_version`. +## Clearfolio attachment view + +`GET /api/projects/:id/attachments/:aid/view` may 302 only to the configured Clearfolio origin or an origin listed in `CLEARFOLIO_ARTIFACT_ORIGINS`. Returned links with userinfo or fragments are rejected. Cross-origin `artifactToken` values are never copied into the Clearfolio viewer host. + ## Spreadsheet exports Every user-controlled CSV cell is neutralized when, after optional leading whitespace, it begins with `=`, `+`, `-`, `@`, or `|`. Export code must not rely on callers to sanitize values. diff --git a/package.json b/package.json index 93430c55..02243e82 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/orchestrator.test.mjs && node tests/unit/orchestrator-coverage.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/orchestrator.test.mjs && node tests/unit/orchestrator-coverage.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 --include=server/orchestrator.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/orchestrator.test.mjs && node tests/unit/orchestrator-coverage.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/orchestrator.test.mjs && node tests/unit/orchestrator-coverage.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/server/clearfolio.mjs b/server/clearfolio.mjs index cb2b5f4b..eee605c9 100644 --- a/server/clearfolio.mjs +++ b/server/clearfolio.mjs @@ -10,6 +10,17 @@ const CLEARFOLIO_JOB_STATUSES = new Set(['PENDING', 'RUNNING', 'SUCCEEDED', 'FAI const MIN_HMAC_SECRET_LENGTH = 32; // WHATWG URL serializes an IPv6 hostname with brackets (`[::1]`). const LOOPBACK_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]']); +const MAX_DOCUMENT_BYTES = 10 * 1024 * 1024; +const MAX_DOCUMENT_NAME_LENGTH = 512; +const MAX_MIME_LENGTH = 255; +const MAX_JOB_ID_LENGTH = 256; +const CONTROL_CHARACTER_PATTERN = /[\u0000-\u001f\u007f]/; + +/** Hard total-request budget for each Clearfolio provider call. */ +export const CLEARFOLIO_REQUEST_TIMEOUT_MS = 15_000; + +/** Maximum successful Clearfolio JSON response bytes read into memory. */ +export const CLEARFOLIO_MAX_RESPONSE_BYTES = 256 * 1024; /** Whether the process uses the explicit in-memory Clearfolio development adapter. */ export const clearfolioMock = process.env.SCOPEWEAVE_DEV === '1' && !CF_URL_INPUT; @@ -182,6 +193,171 @@ function isClearfolioJobStatus(value) { return typeof value === 'string' && CLEARFOLIO_JOB_STATUSES.has(value); } +/** + * Validate document metadata and bytes before allocating Blob/FormData objects. + * + * ScopeWeave's browser/API attachment ceiling is 10 MiB, so the provider adapter + * never accepts a larger in-process document than the caller can legitimately + * upload. Empty MIME is preserved as the existing application/octet-stream + * fallback. + * + * @param {unknown} document - Untrusted adapter input. + * @returns {{name:string,mime:string,bytes:Uint8Array}} Validated document input. + * @throws {Error} If metadata or bytes are malformed or outside the bounded contract. + */ +function validateDocument(document) { + if (!isJsonRecord(document)) throw new Error('clearfolio document invalid'); + const { name, mime, bytes } = document; + if ( + typeof name !== 'string' + || name.trim().length === 0 + || name.length > MAX_DOCUMENT_NAME_LENGTH + || CONTROL_CHARACTER_PATTERN.test(name) + || typeof mime !== 'string' + || mime.length > MAX_MIME_LENGTH + || CONTROL_CHARACTER_PATTERN.test(mime) + || !(bytes instanceof Uint8Array) + || bytes.byteLength > MAX_DOCUMENT_BYTES + ) { + throw new Error('clearfolio document invalid'); + } + return { name, mime, bytes }; +} + +/** + * Canonicalize and bound a provider job identifier before it reaches a URL. + * + * @param {unknown} jobId - Persisted or provider-returned job identifier. + * @returns {string} Trimmed non-empty identifier no longer than 256 characters. + * @throws {Error} If the identifier is unusable. + */ +function validateJobId(jobId) { + if (typeof jobId !== 'string') throw new Error('clearfolio job id invalid'); + const canonical = jobId.trim(); + if ( + canonical.length === 0 + || canonical.length > MAX_JOB_ID_LENGTH + || CONTROL_CHARACTER_PATTERN.test(canonical) + ) { + throw new Error('clearfolio job id invalid'); + } + return canonical; +} + +/** + * Compose an optional caller cancellation signal with the hard provider budget. + * + * @param {AbortSignal|undefined} callerSignal - Optional upstream cancellation signal. + * @returns {AbortSignal} Signal that aborts on caller cancellation or total timeout. + */ +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]); +} + +/** + * Cancel an unread provider response body before returning a fixed rejection. + * + * Undici-backed fetch responses must be consumed or cancelled so rejected + * downstream bodies cannot strand connection-pool resources. Cancellation + * failures are deliberately hidden because the operation-level error remains + * the authoritative, non-secret client and operator signal. + * + * @param {Response} response - Provider response whose payload must remain unread. + * @param {string} errorMessage - Fixed non-secret error to throw after cancellation. + * @returns {Promise} Promise that always rejects with the fixed error. + */ +async function rejectProviderResponse(response, errorMessage) { + try { + if (response?.body && typeof response.body.cancel === 'function') { + await response.body.cancel(); + } + } catch { + // The fixed operation-level rejection remains authoritative. + } + throw new Error(errorMessage); +} + +/** + * Parse one successful provider JSON response with media-type and byte bounds. + * + * Content-Length is treated only as an early rejection hint; the body stream is + * independently counted so omitted or dishonest length headers cannot bypass the + * memory ceiling. Invalid UTF-8, JSON, stream errors, and cancellation are + * collapsed to one operation-level message. + * + * @param {Response} response - Successful fetch response. + * @param {string} invalidMessage - Fixed operation-level validation error. + * @returns {Promise} Parsed JSON value. + * @throws {Error} If media type, declared/streamed size, UTF-8, or JSON is invalid. + */ +async function readBoundedJson(response, invalidMessage) { + const contentType = response?.headers?.get?.('content-type'); + if ( + typeof contentType !== 'string' + || contentType.split(';', 1)[0].trim().toLowerCase() !== 'application/json' + ) { + return rejectProviderResponse(response, invalidMessage); + } + + const contentLength = response.headers.get('content-length'); + if (contentLength !== null) { + if (!/^\d+$/.test(contentLength)) return rejectProviderResponse(response, invalidMessage); + const declaredBytes = Number(contentLength); + if (!Number.isSafeInteger(declaredBytes) || declaredBytes > CLEARFOLIO_MAX_RESPONSE_BYTES) { + return rejectProviderResponse(response, invalidMessage); + } + } + + if (!response.body || typeof response.body.getReader !== 'function') { + return rejectProviderResponse(response, invalidMessage); + } + + const reader = response.body.getReader(); + const chunks = []; + let totalBytes = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (!(value instanceof Uint8Array)) throw new Error(invalidMessage); + totalBytes += value.byteLength; + if (totalBytes > CLEARFOLIO_MAX_RESPONSE_BYTES) { + try { await reader.cancel(); } catch { /* validation remains authoritative */ } + throw new Error(invalidMessage); + } + chunks.push(value); + } + } catch (error) { + if (error?.message === invalidMessage) throw error; + throw new Error(invalidMessage); + } finally { + try { reader.releaseLock(); } catch { /* no observable effect */ } + } + + if (totalBytes === 0) throw new Error(invalidMessage); + const bytes = new Uint8Array(totalBytes); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + + let text; + try { + text = new TextDecoder('utf-8', { fatal: true }).decode(bytes); + } catch { + throw new Error(invalidMessage); + } + try { + return JSON.parse(text); + } catch { + throw new Error(invalidMessage); + } +} + // ---- explicit development-only mock store (restart discards it) ---- const mockDocs = new Map(); // jobId -> { name, mime, bytes } let mockSeq = 0; @@ -206,37 +382,46 @@ export const mockArtifact = (jobId) => (clearfolioMock ? mockDocs.get(jobId) || * @returns {Promise<{jobId:string,status:string}>} Downstream job identity and initial status. * @throws {Error} If Clearfolio is unavailable, rejects the request, or returns a malformed response. */ -export async function submitJob(orgId, userId, { name, mime, bytes }) { +export async function submitJob(orgId, userId, document) { + const validatedDocument = validateDocument(document); const configuration = clearfolioConfiguration(); if (configuration.mock) { const jobId = `mockcf-${++mockSeq}`; - mockDocs.set(jobId, { name, mime, bytes }); + mockDocs.set(jobId, validatedDocument); return { jobId, status: 'SUCCEEDED' }; } const form = new FormData(); - form.append('file', new Blob([bytes], { type: mime || 'application/octet-stream' }), name); + form.append( + 'file', + 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) throw new Error(`clearfolio submit failed (${res.status})`); - const data = await res.json().catch(() => null); + 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' - || data.jobId.trim().length === 0 - || !isClearfolioJobStatus(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: data.jobId.trim(), status }; + return { jobId, status }; } /** @@ -255,32 +440,110 @@ export async function submitJob(orgId, userId, { name, mime, bytes }) { * @throws {Error} If Clearfolio is unavailable, rejects the request, or returns a malformed status. */ export async function jobStatus(orgId, userId, jobId, { signal } = {}) { + const canonicalJobId = validateJobId(jobId); const configuration = clearfolioConfiguration(); - if (configuration.mock) return mockDocs.has(jobId) ? 'SUCCEEDED' : 'FAILED'; + if (configuration.mock) return mockDocs.has(canonicalJobId) ? 'SUCCEEDED' : 'FAILED'; let res; try { - res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs/${encodeURIComponent(jobId)}`, { + res = await fetch(`${configuration.baseUrl}/api/v1/convert/jobs/${encodeURIComponent(canonicalJobId)}`, { headers: tenantHeaders(orgId, userId, configuration.secret), - signal, + signal: providerSignal(signal), + redirect: 'error', }); } catch { throw new Error('clearfolio status unavailable'); } - if (!res.ok) throw new Error(`clearfolio status failed (${res.status})`); - const data = await res.json().catch(() => null); + 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; } +/** + * Parse the reviewed artifact-origin allowlist used for attachment-view redirects. + * + * Each entry must be an absolute origin URL. Production entries require HTTPS + * and reject credentials, query strings, fragments, and paths so the allowlist + * cannot become an arbitrary request prefix. HTTP is limited to explicit + * development-mode loopback hosts. An empty setting trusts only the configured + * Clearfolio origin. + * + * @returns {string[]} Canonical WHATWG origins. + * @throws {ClearfolioConfigurationError} If any configured origin is unsafe or ambiguous. + */ +function artifactOriginAllowlist() { + const input = String(process.env.CLEARFOLIO_ARTIFACT_ORIGINS || '').trim(); + if (!input) return []; + const origins = []; + const seen = new Set(); + for (const part of input.split(',')) { + const raw = part.trim(); + if (!raw) continue; + let url; + try { + url = new URL(raw); + } catch { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS must be a comma-separated list of absolute origin URLs. Set reviewed HTTPS CDN origins, or leave the setting empty to allow only the configured Clearfolio origin.', + ); + } + if (url.username || url.password) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS entries must not contain credentials.', + ); + } + if (url.search) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS entries must not contain a query string.', + ); + } + if (url.hash) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS entries must not contain a fragment.', + ); + } + if (url.pathname !== '/') { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS entries must identify an origin without a path.', + ); + } + if (!['https:', 'http:'].includes(url.protocol)) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS entries must use HTTP or HTTPS.', + ); + } + const isLoopback = LOOPBACK_HOSTNAMES.has(url.hostname); + if (url.protocol === 'http:' && !(process.env.SCOPEWEAVE_DEV === '1' && isLoopback)) { + throw new ClearfolioConfigurationError( + 'clearfolio_artifact_origins_invalid', + 'CLEARFOLIO_ARTIFACT_ORIGINS production entries require HTTPS.', + ); + } + if (!seen.has(url.origin)) { + seen.add(url.origin); + origins.push(url.origin); + } + } + return origins; +} + /** * Issue a viewable artifact URL for a completed Clearfolio job. * * Same-origin `artifactToken` values may be translated into the trusted viewer - * route. Token-bearing links from another origin are rejected until an explicit - * reviewed artifact-origin allowlist exists; tokens are never transplanted or - * returned to an unreviewed cross-origin host. + * route. Cross-origin links are returned only when the origin appears in the + * reviewed `CLEARFOLIO_ARTIFACT_ORIGINS` allowlist; tokens on those hosts stay + * on that origin and are never transplanted into the Clearfolio viewer. + * Credentials and fragments are rejected before the attachment-view 302 target + * is returned. * * @param {string|number} orgId - ScopeWeave organization identifier. * @param {string|number} userId - Requesting ScopeWeave user identifier. @@ -289,19 +552,22 @@ export async function jobStatus(orgId, userId, jobId, { signal } = {}) { * @throws {Error} If Clearfolio is unavailable, rejects the request, or returns an invalid link. */ export async function artifactUrl(orgId, userId, jobId) { + const canonicalJobId = validateJobId(jobId); const configuration = clearfolioConfiguration(); - if (configuration.mock) return `/api/mock-clearfolio/${encodeURIComponent(jobId)}`; + if (configuration.mock) return `/api/mock-clearfolio/${encodeURIComponent(canonicalJobId)}`; let res; try { - res = await fetch(`${configuration.baseUrl}/api/v1/viewer/${encodeURIComponent(jobId)}/artifact-links`, { + 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) throw new Error(`clearfolio artifact-link failed (${res.status})`); - const data = await res.json().catch(() => null); + 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) { @@ -320,13 +586,19 @@ export async function artifactUrl(orgId, userId, jobId) { if (url.protocol !== 'https:' && !allowsHttp) { throw new Error('clearfolio artifact-link response invalid'); } + if (url.username || url.password || url.hash) { + throw new Error('clearfolio artifact-link response invalid'); + } + + const allowlist = artifactOriginAllowlist(); + const sameOrigin = url.origin === clearfolioUrl.origin; + if (!sameOrigin && !allowlist.includes(url.origin)) { + throw new Error('clearfolio artifact-link response invalid'); + } const token = url.searchParams.get('artifactToken'); - if (token) { - if (url.origin !== clearfolioUrl.origin) { - throw new Error('clearfolio artifact-link response invalid'); - } - return `${configuration.baseUrl}/viewer/${encodeURIComponent(jobId)}?artifactToken=${encodeURIComponent(token)}`; + if (token && sameOrigin) { + return `${configuration.baseUrl}/viewer/${encodeURIComponent(canonicalJobId)}?artifactToken=${encodeURIComponent(token)}`; } return url.href; } diff --git a/tests/unit/clearfolio-adapter-mock-hmac.test.mjs b/tests/unit/clearfolio-adapter-mock-hmac.test.mjs index fb48b570..eba5783e 100644 --- a/tests/unit/clearfolio-adapter-mock-hmac.test.mjs +++ b/tests/unit/clearfolio-adapter-mock-hmac.test.mjs @@ -7,6 +7,11 @@ async function freshModule(label) { return import(`../../server/clearfolio.mjs?${label}-${Date.now()}-${Math.random()}`); } +const jsonResponse = (value) => new Response(JSON.stringify(value), { + status: 200, + headers: { 'content-type': 'application/json; charset=utf-8' }, +}); + test('unconfigured production fails closed instead of creating fake conversions', async () => { delete process.env.SCOPEWEAVE_DEV; delete process.env.CLEARFOLIO_URL; @@ -89,11 +94,11 @@ test('production URL and HMAC configuration rejects ambiguous or unsafe input', process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; const loopback = await freshModule('development-loopback-http'); const originalFetch = globalThis.fetch; - globalThis.fetch = async () => ({ - ok: true, - status: 200, - json: async () => ({ status: 'RUNNING' }), - }); + globalThis.fetch = async (_url, options) => { + assert.equal(options.redirect, 'error'); + assert.ok(options.signal instanceof AbortSignal); + return jsonResponse({ status: 'RUNNING' }); + }; try { assert.equal(await loopback.jobStatus(1, 2, 'job-1'), 'RUNNING'); @@ -102,7 +107,7 @@ test('production URL and HMAC configuration rejects ambiguous or unsafe input', assert.equal( await ipv6Loopback.jobStatus(1, 2, 'job-1'), 'RUNNING', - 'explicit development mode accepts the IPv6 loopback origin documented by the adapter', + 'explicit development mode accepts the WHATWG-serialized IPv6 loopback origin', ); } finally { globalThis.fetch = originalFetch; @@ -124,11 +129,7 @@ test('Clearfolio tenant claim headers use the documented HMAC contract', async ( globalThis.fetch = async (url, options) => { observedUrl = String(url); observedOptions = options; - return { - ok: true, - status: 200, - json: async () => ({ status: 'RUNNING' }), - }; + return jsonResponse({ status: 'RUNNING' }); }; try { @@ -139,6 +140,8 @@ test('Clearfolio tenant claim headers use the documented HMAC contract', async ( observedUrl, 'https://clearfolio.example/api/v1/convert/jobs/signed-job', ); + assert.equal(observedOptions.redirect, 'error'); + assert.ok(observedOptions.signal instanceof AbortSignal); const issuedAt = '1750000000'; assert.equal(observedOptions.headers['X-Clearfolio-Tenant-Id'], 'sw-org-21'); diff --git a/tests/unit/clearfolio-artifact-origin.test.mjs b/tests/unit/clearfolio-artifact-origin.test.mjs new file mode 100644 index 00000000..358370b3 --- /dev/null +++ b/tests/unit/clearfolio-artifact-origin.test.mjs @@ -0,0 +1,158 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +const HMAC_SECRET = 'clearfolio-shared-secret-32-bytes!!'; + +async function freshModule(label) { + return import(`../../server/clearfolio.mjs?artifact-origin-${label}-${Date.now()}-${Math.random()}`); +} + +function jsonResponse(value) { + return new Response(JSON.stringify(value), { + status: 200, + headers: { 'content-type': 'application/json; charset=utf-8' }, + }); +} + +async function withArtifactLink(moduleName, link, run) { + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => jsonResponse({ artifactUrl: link }); + try { + const { artifactUrl } = await freshModule(moduleName); + return await run(artifactUrl); + } finally { + globalThis.fetch = originalFetch; + } +} + +function configureProductionProvider() { + delete process.env.SCOPEWEAVE_DEV; + process.env.CLEARFOLIO_URL = 'https://clearfolio.example'; + process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; +} + +test.after(() => { + delete process.env.CLEARFOLIO_URL; + delete process.env.CLEARFOLIO_HMAC_SECRET; + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + delete process.env.SCOPEWEAVE_DEV; +}); + +test('tokenless cross-origin artifact links fail closed without a reviewed origin allowlist', async () => { + configureProductionProvider(); + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + await withArtifactLink('unlisted-cdn', 'https://cdn.example/file.pdf', async (artifactUrl) => { + await assert.rejects( + () => artifactUrl(4, 5, 'job-1'), + /clearfolio artifact-link response invalid/, + ); + }); +}); + +test('artifact links with credentials or fragments are rejected before any redirect target is returned', async () => { + configureProductionProvider(); + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = 'https://cdn.example'; + for (const [label, link] of [ + ['userinfo', 'https://user:pass@cdn.example/file.pdf'], + ['fragment', 'https://cdn.example/file.pdf#phish'], + ]) { + await withArtifactLink(label, link, async (artifactUrl) => { + await assert.rejects( + () => artifactUrl(4, 5, 'job-1'), + /clearfolio artifact-link response invalid/, + `${label} must never become an attachment-view redirect`, + ); + }); + } +}); + +test('same-origin tokenless and token-bearing viewer links remain usable without an allowlist', async () => { + configureProductionProvider(); + delete process.env.CLEARFOLIO_ARTIFACT_ORIGINS; + await withArtifactLink('same-origin-relative', '/signed/file.pdf', async (artifactUrl) => { + assert.equal(await artifactUrl(4, 5, 'job-1'), 'https://clearfolio.example/signed/file.pdf'); + }); + await withArtifactLink( + 'same-origin-token', + 'https://clearfolio.example/file.pdf?artifactToken=same%20origin', + async (artifactUrl) => { + assert.equal( + await artifactUrl(4, 5, 'job-1'), + 'https://clearfolio.example/viewer/job-1?artifactToken=same%20origin', + ); + }, + ); +}); + +test('reviewed CDN origins may be returned only when listed and never transplanted into the viewer', async () => { + configureProductionProvider(); + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = 'https://cdn.example,, https://cdn.example, https://files.example:8443'; + await withArtifactLink('allowlisted-cdn', 'https://cdn.example/file.pdf', async (artifactUrl) => { + assert.equal(await artifactUrl(4, 5, 'job-1'), 'https://cdn.example/file.pdf'); + }); + await withArtifactLink( + 'allowlisted-cdn-token', + 'https://cdn.example/file.pdf?artifactToken=cdn-token', + async (artifactUrl) => { + assert.equal( + await artifactUrl(4, 5, 'job-1'), + 'https://cdn.example/file.pdf?artifactToken=cdn-token', + 'an allowlisted CDN token stays on that origin instead of moving into the Clearfolio viewer', + ); + }, + ); + await withArtifactLink( + 'allowlisted-port', + 'https://files.example:8443/export.pdf', + async (artifactUrl) => { + assert.equal(await artifactUrl(4, 5, 'job-1'), 'https://files.example:8443/export.pdf'); + }, + ); + await withArtifactLink( + 'unlisted-other-cdn', + 'https://other-cdn.example/file.pdf', + async (artifactUrl) => { + await assert.rejects( + () => artifactUrl(4, 5, 'job-1'), + /clearfolio artifact-link response invalid/, + ); + }, + ); +}); + +test('explicit development mode may allowlist a second loopback HTTP origin', async () => { + process.env.SCOPEWEAVE_DEV = '1'; + process.env.CLEARFOLIO_URL = 'http://127.0.0.1:8080'; + process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = 'http://127.0.0.1:9090'; + await withArtifactLink( + 'dev-loopback-allowlist', + 'http://127.0.0.1:9090/file.pdf', + async (artifactUrl) => { + assert.equal(await artifactUrl(4, 5, 'job-http'), 'http://127.0.0.1:9090/file.pdf'); + }, + ); +}); + +test('CLEARFOLIO_ARTIFACT_ORIGINS rejects unsafe or ambiguous origin entries before returning a view link', async () => { + configureProductionProvider(); + const cases = [ + ['https://user:pass@cdn.example', 'clearfolio_artifact_origins_invalid'], + ['https://cdn.example/path', 'clearfolio_artifact_origins_invalid'], + ['https://cdn.example?x=1', 'clearfolio_artifact_origins_invalid'], + ['https://cdn.example#frag', 'clearfolio_artifact_origins_invalid'], + ['http://cdn.example', 'clearfolio_artifact_origins_invalid'], + ['ftp://cdn.example', 'clearfolio_artifact_origins_invalid'], + ['not-a-url', 'clearfolio_artifact_origins_invalid'], + ]; + for (const [origins, code] of cases) { + process.env.CLEARFOLIO_ARTIFACT_ORIGINS = origins; + await withArtifactLink(`invalid-allowlist-${code}`, 'https://cdn.example/file.pdf', async (artifactUrl) => { + await assert.rejects( + () => artifactUrl(4, 5, 'job-1'), + (error) => error.code === code, + `${origins} should fail closed with ${code}`, + ); + }); + } +}); diff --git a/tests/unit/clearfolio-provider-boundary.test.mjs b/tests/unit/clearfolio-provider-boundary.test.mjs new file mode 100644 index 00000000..f8628094 --- /dev/null +++ b/tests/unit/clearfolio-provider-boundary.test.mjs @@ -0,0 +1,214 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +const HMAC_SECRET = 'clearfolio-shared-secret-32-bytes!!'; +process.env.CLEARFOLIO_URL = 'https://clearfolio.example'; +process.env.CLEARFOLIO_HMAC_SECRET = HMAC_SECRET; +delete process.env.SCOPEWEAVE_DEV; + +const originalFetch = globalThis.fetch; +const calls = []; +let responder; +globalThis.fetch = async (url, options = {}) => { + calls.push({ url: String(url), options }); + if (!responder) throw new Error('test responder is not configured'); + return responder(url, options); +}; + +const { + CLEARFOLIO_MAX_RESPONSE_BYTES, + CLEARFOLIO_REQUEST_TIMEOUT_MS, + artifactUrl, + jobStatus, + submitJob, +} = await import(`../../server/clearfolio.mjs?provider-boundary=${Date.now()}`); + +const jsonResponse = (value, init = {}) => new Response(JSON.stringify(value), { + status: init.status ?? 200, + headers: { + 'content-type': init.contentType ?? 'application/json; charset=utf-8', + ...(init.headers || {}), + }, +}); + +function useResponse(value, init = {}) { + responder = async () => jsonResponse(value, init); +} + +test.after(() => { + globalThis.fetch = originalFetch; + delete process.env.CLEARFOLIO_URL; + delete process.env.CLEARFOLIO_HMAC_SECRET; +}); + +test('provider requests disable redirects and carry a bounded total-request signal', async () => { + useResponse({ status: 'RUNNING' }); + const before = calls.length; + assert.equal(await jobStatus(1, 2, 'job-1'), 'RUNNING'); + assert.equal(calls.length, before + 1); + const { options } = calls.at(-1); + assert.equal(options.redirect, 'error'); + assert.ok(options.signal instanceof AbortSignal); + assert.equal(options.signal.aborted, false); + assert.equal(Number.isSafeInteger(CLEARFOLIO_REQUEST_TIMEOUT_MS), true); + assert.equal(CLEARFOLIO_REQUEST_TIMEOUT_MS > 0 && CLEARFOLIO_REQUEST_TIMEOUT_MS <= 30_000, true); +}); + +test('status response requires JSON media type before parsing', async () => { + useResponse({ status: 'RUNNING' }, { contentType: 'text/plain' }); + await assert.rejects( + () => jobStatus(1, 2, 'job-1'), + /clearfolio status response invalid/, + ); +}); + +test('declared and streamed provider response bodies are bounded', async () => { + assert.equal(Number.isSafeInteger(CLEARFOLIO_MAX_RESPONSE_BYTES), true); + assert.equal(CLEARFOLIO_MAX_RESPONSE_BYTES >= 1024 && CLEARFOLIO_MAX_RESPONSE_BYTES <= 1024 * 1024, true); + + useResponse({ status: 'RUNNING' }, { + headers: { 'content-length': String(CLEARFOLIO_MAX_RESPONSE_BYTES + 1) }, + }); + await assert.rejects( + () => jobStatus(1, 2, 'job-1'), + /clearfolio status response invalid/, + ); + + responder = async () => new Response( + new Uint8Array(CLEARFOLIO_MAX_RESPONSE_BYTES + 1), + { headers: { 'content-type': 'application/json' } }, + ); + await assert.rejects( + () => jobStatus(1, 2, 'job-1'), + /clearfolio status response invalid/, + ); +}); + +test('caller cancellation remains composed with the provider request budget', async () => { + const controller = new AbortController(); + controller.abort(new Error('caller cancelled with private detail')); + responder = async (_url, options) => { + assert.equal(options.signal.aborted, true); + throw options.signal.reason; + }; + await assert.rejects( + () => jobStatus(1, 2, 'job-1', { signal: controller.signal }), + (error) => { + assert.equal(error.message, 'clearfolio status unavailable'); + assert.doesNotMatch(error.message, /private detail/); + return true; + }, + ); +}); + +test('document validation fails before Blob, FormData, or provider transport', async () => { + const before = calls.length; + const invalidDocuments = [ + null, + { name: '', mime: 'text/plain', bytes: Buffer.from('x') }, + { name: 'x'.repeat(513), mime: 'text/plain', bytes: Buffer.from('x') }, + { name: 'x.txt', mime: 'x'.repeat(256), bytes: Buffer.from('x') }, + { name: 'x.txt', mime: 'text/plain', bytes: 'not-bytes' }, + { name: 'x.txt', mime: 'text/plain', bytes: new Uint8Array(10 * 1024 * 1024 + 1) }, + ]; + for (const document of invalidDocuments) { + await assert.rejects( + () => submitJob(1, 2, document), + /clearfolio document invalid/, + ); + } + assert.equal(calls.length, before, 'invalid documents never reach provider transport'); +}); + +test('provider job identifiers are bounded before URL construction', async () => { + const before = calls.length; + for (const operation of [ + () => jobStatus(1, 2, 'x'.repeat(257)), + () => artifactUrl(1, 2, 'x'.repeat(257)), + ]) { + await assert.rejects(operation, /clearfolio job id invalid/); + } + assert.equal(calls.length, before, 'oversized job identifiers never reach provider transport'); +}); + +test('valid submit response remains compatible with the bounded transport', async () => { + useResponse({ jobId: ' job-2 ', status: 'PENDING' }); + assert.deepEqual( + await submitJob(7, 9, { name: 'status.txt', mime: 'text/plain', bytes: Buffer.from('status') }), + { jobId: 'job-2', status: 'PENDING' }, + ); + const { options } = calls.at(-1); + assert.equal(options.redirect, 'error'); + assert.ok(options.signal instanceof AbortSignal); + assert.ok(options.body instanceof FormData); +}); + +test('non-success provider responses cancel unread bodies without parsing downstream payloads', async () => { + let cancelledBodies = 0; + const privatePayload = 'private downstream payload that must remain unread'; + responder = async () => new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(privatePayload)); + }, + cancel() { + cancelledBodies += 1; + }, + }), + { + status: 503, + headers: { 'content-type': 'application/json; charset=utf-8' }, + }, + ); + + const operations = [ + { + run: () => submitJob(1, 2, { name: 'x.txt', mime: 'text/plain', bytes: Buffer.from('x') }), + expected: 'clearfolio submit failed (503)', + }, + { + run: () => jobStatus(1, 2, 'job-1'), + expected: 'clearfolio status failed (503)', + }, + { + run: () => artifactUrl(1, 2, 'job-1'), + expected: 'clearfolio artifact-link failed (503)', + }, + ]; + + for (const [index, operation] of operations.entries()) { + await assert.rejects( + operation.run, + (error) => { + assert.equal(error.message, operation.expected); + assert.doesNotMatch(error.message, /private downstream payload/); + return true; + }, + ); + assert.equal(cancelledBodies, index + 1, 'each rejected response body is explicitly cancelled'); + } + + responder = async () => new Response(null, { status: 503 }); + await assert.rejects( + () => jobStatus(1, 2, 'job-1'), + /clearfolio status failed \(503\)/, + ); + assert.equal(cancelledBodies, 3, 'a response without a body needs no cancellation'); + + responder = async () => new Response( + new ReadableStream({ + cancel() { + throw new Error('private cancel failure'); + }, + }), + { status: 503 }, + ); + await assert.rejects( + () => artifactUrl(1, 2, 'job-1'), + (error) => { + assert.equal(error.message, 'clearfolio artifact-link failed (503)'); + assert.doesNotMatch(error.message, /private cancel failure/); + return true; + }, + ); +}); diff --git a/tests/unit/clearfolio-status-signal.test.mjs b/tests/unit/clearfolio-status-signal.test.mjs index 0f30e231..e3171421 100644 --- a/tests/unit/clearfolio-status-signal.test.mjs +++ b/tests/unit/clearfolio-status-signal.test.mjs @@ -14,16 +14,27 @@ globalThis.fetch = async (url, options = {}) => { observedUrl = String(url); observedOptions = options; if (downstreamError) throw downstreamError; - return downstreamResponse; + return downstreamResponse(); }; const { artifactUrl, jobStatus, submitJob } = await import( '../../server/clearfolio.mjs?downstream-contract-test=1' ); -function setResponse({ ok = true, status = 200, json }) { +function setResponse({ status = 200, json }) { downstreamError = undefined; - downstreamResponse = { ok, status, json }; + downstreamResponse = async () => { + let body; + try { + body = JSON.stringify(await json()); + } catch { + body = '{'; + } + return new Response(body, { + status, + headers: { 'content-type': 'application/json; charset=utf-8' }, + }); + }; } function setNetworkError(error) { @@ -52,7 +63,9 @@ test('jobStatus enforces endpoint, signal, transport, HTTP, and status contracts const status = await jobStatus(1, 2, 'job-1', { signal: controller.signal }); assert.equal(status, 'RUNNING'); assert.equal(observedUrl, 'https://clearfolio.example/api/v1/convert/jobs/job-1'); - assert.equal(observedOptions.signal, controller.signal); + assert.ok(observedOptions.signal instanceof AbortSignal); + assert.notEqual(observedOptions.signal, controller.signal, 'caller signal is composed with provider timeout'); + assert.equal(observedOptions.redirect, 'error'); setNetworkError(new Error('connect ECONNREFUSED https://private-clearfolio.internal')); await expectSanitizedFailure( @@ -62,7 +75,6 @@ test('jobStatus enforces endpoint, signal, transport, HTTP, and status contracts ); setResponse({ - ok: false, status: 503, json: async () => ({ message: 'sensitive downstream text' }), }); @@ -113,7 +125,6 @@ test('submitJob rejects transport details and malformed successful responses', a ); setResponse({ - ok: false, status: 422, json: async () => ({ message: 'tenant-internal rejection detail' }), }); @@ -124,6 +135,8 @@ test('submitJob rejects transport details and malformed successful responses', a ); assert.equal(observedUrl, 'https://clearfolio.example/api/v1/convert/jobs'); assert.equal(observedOptions.method, 'POST'); + assert.equal(observedOptions.redirect, 'error'); + assert.ok(observedOptions.signal instanceof AbortSignal); assert.ok(observedOptions.body instanceof FormData); const malformedPayloads = [ @@ -175,7 +188,6 @@ test('artifactUrl validates links and never exposes transport or response text', ); setResponse({ - ok: false, status: 502, json: async () => ({ message: 'signed URL service secret detail' }), }); @@ -189,6 +201,8 @@ test('artifactUrl validates links and never exposes transport or response text', 'https://clearfolio.example/api/v1/viewer/job-1/artifact-links', ); assert.equal(observedOptions.method, 'POST'); + assert.equal(observedOptions.redirect, 'error'); + assert.ok(observedOptions.signal instanceof AbortSignal); const malformedPayloads = [ { @@ -231,9 +245,9 @@ test('artifactUrl validates links and never exposes transport or response text', ); setResponse({ json: async () => ({ url: 'https://cdn.example/file.pdf' }) }); - assert.equal( - await artifactUrl(4, 5, 'job-1'), - 'https://cdn.example/file.pdf', + await expectSanitizedFailure( + () => artifactUrl(4, 5, 'job-1'), + 'clearfolio artifact-link response invalid', ); setResponse({ diff --git a/tests/unit/coverage-script-contract.test.mjs b/tests/unit/coverage-script-contract.test.mjs index 149440e5..d9557490 100644 --- a/tests/unit/coverage-script-contract.test.mjs +++ b/tests/unit/coverage-script-contract.test.mjs @@ -39,6 +39,11 @@ assert.match( /tests\/unit\/clearfolio-status-signal\.test\.mjs/, 'the Clearfolio signal and HTTP failure regression executes under c8', ); +assert.match( + scripts['test:coverage:cases'], + /tests\/unit\/clearfolio-artifact-origin\.test\.mjs/, + 'the Clearfolio artifact-origin allowlist regression executes under c8', +); assert.doesNotMatch( scripts['test:coverage:cases'], /npm run (?:coverage|test:coverage)(?:\s|$)/,