FEATURE: Add content scanning and abuse controls - #37
bmdavis419 wants to merge 11 commits into
Conversation
|
Warning Review limit reached
On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file. Or wait 33 minutes for your next included review. View limit detailsLimit details: You’ve used all 6 included reviews currently available. Your 49 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (110)
Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| yield* sql` | ||
| UPDATE files | ||
| SET public = ${visibility.public}, updated_at = ${updatedAt} | ||
| SET public = ${isPublicNow}, publish_pending = ${hold}, |
There was a problem hiding this comment.
🟠 High files/mutations.ts:61
Cancelling a held publish in setVisibility does not prevent the in-flight scan from later setting public = true, so a file the user made private is exposed again. The scanner’s publish update must also require publish_pending to still be true (or otherwise invalidate the scan when this flag is cleared).
Also found in 1 other location(s)
apps/web/src/lib/server/services/scanner.ts:280
The publish update checks the version and quarantine state but not
publish_pending. If a user cancels a pending publish by making the file private while its scan is running,setVisibilityclearspublish_pending; this stale scan then still executes this update and makes the file public. The user’s explicit privacy change is therefore undone and private content is exposed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/lib/server/services/files/mutations.ts around line 61:
Cancelling a held publish in `setVisibility` does not prevent the in-flight scan from later setting `public = true`, so a file the user made private is exposed again. The scanner’s publish update must also require `publish_pending` to still be true (or otherwise invalidate the scan when this flag is cleared).
Also found in 1 other location(s):
- apps/web/src/lib/server/services/scanner.ts:280 -- The publish update checks the version and quarantine state but not `publish_pending`. If a user cancels a pending publish by making the file private while its scan is running, `setVisibility` clears `publish_pending`; this stale scan then still executes this update and makes the file public. The user’s explicit privacy change is therefore undone and private content is exposed.
| scan: received, | ||
| // The scanner records its own outcome (a verdict row, a re-sent | ||
| // poll); only storage trouble asks for a redelivery. | ||
| scan: (job) => scanner.runOne(job).pipe(Effect.as('done')), |
There was a problem hiding this comment.
🟠 High jobs/consumer.ts:100
scan acknowledges the job as 'done' even when scanner.runOne returns Polling after jobs.trySend fails, so the only follow-up URL-scan poll is lost. Because trySend only logs the enqueue error and Lifecycle has no reconciliation sweep, held publishes remain pending indefinitely and already-public files are never finalized or quarantined. Preserve the scanner outcome (or propagate the enqueue failure) so the message is retried when the poll cannot be queued.
Also found in 3 other location(s)
apps/web/src/lib/server/services/files/mutations.ts:70
sendScanJobat line 70 usesjobs.trySend, which deliberately swallows a queue-send failure. This mutation leaves verified-org files withpublic = falseandpublish_pending = true, but the lifecycle only runs purge/index/site work and there is no scan reconciliation path. Therefore a transient queue outage permanently leaves a requested publish unavailable rather than retrying it.
apps/web/src/lib/server/services/scanner.ts:511
The initial URL-scan pass submits the URLs and then uses
jobs.trySendfor the only polling job.trySendswallows queue-send failures, while this service persists no pending poll state for maintenance to reconstruct. If that enqueue fails, the consumed job is acknowledged: a verified file remains held forever, and an already-public file is never quarantined even if the submitted URL scan later reports malicious.
apps/web/src/lib/server/services/sites/sessions.ts:413
Using
jobs.trySendfor the only scan job silently drops the job during a queue-send outage. A verified org's commit has already storedpublic = false, publish_pending = true, and there is no scanner lifecycle sweep or other reconciliation that re-enqueues held rows (the lifecycle only sweeps sites, indexing, and file purges). The site therefore remains permanently unavailable until manual intervention instead of eventually being scanned and published.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/lib/server/jobs/consumer.ts around line 100:
`scan` acknowledges the job as `'done'` even when `scanner.runOne` returns `Polling` after `jobs.trySend` fails, so the only follow-up URL-scan poll is lost. Because `trySend` only logs the enqueue error and `Lifecycle` has no reconciliation sweep, held publishes remain pending indefinitely and already-public files are never finalized or quarantined. Preserve the scanner outcome (or propagate the enqueue failure) so the message is retried when the poll cannot be queued.
Also found in 3 other location(s):
- apps/web/src/lib/server/services/files/mutations.ts:70 -- `sendScanJob` at line 70 uses `jobs.trySend`, which deliberately swallows a queue-send failure. This mutation leaves verified-org files with `public = false` and `publish_pending = true`, but the lifecycle only runs purge/index/site work and there is no scan reconciliation path. Therefore a transient queue outage permanently leaves a requested publish unavailable rather than retrying it.
- apps/web/src/lib/server/services/scanner.ts:511 -- The initial URL-scan pass submits the URLs and then uses `jobs.trySend` for the only polling job. `trySend` swallows queue-send failures, while this service persists no pending poll state for maintenance to reconstruct. If that enqueue fails, the consumed job is acknowledged: a verified file remains held forever, and an already-public file is never quarantined even if the submitted URL scan later reports malicious.
- apps/web/src/lib/server/services/sites/sessions.ts:413 -- Using `jobs.trySend` for the only scan job silently drops the job during a queue-send outage. A verified org's commit has already stored `public = false, publish_pending = true`, and there is no scanner lifecycle sweep or other reconciliation that re-enqueues held rows (the lifecycle only sweeps sites, indexing, and file purges). The site therefore remains permanently unavailable until manual intervention instead of eventually being scanned and published.
| ); | ||
| const trust = recover( | ||
| 'trust', | ||
| Effect.suspend(() => promoteEstablished(sql, new Date())), |
There was a problem hiding this comment.
🟠 High services/lifecycle.ts:101
lifecycle.trust promotes a long-lived free organization to established on the next maintenance tick after it upgrades to a paid plan, instead of waiting 14 days on the paid plan. promoteEstablished uses orgs.created_at for the cutoff, so it bypasses the intended verified-organization scan hold; base eligibility on the paid-plan transition time instead.
Also found in 2 other location(s)
apps/web/src/lib/server/trust.ts:59
promoteEstablishedcomparescreated_atwith the 14-day cutoff, so an old free org that upgrades today is promoted toestablishedon the next sweep instead of after 14 days on a paid plan. This bypasses the intended verified-org scan-before-publish period immediately after upgrading.
apps/web/src/routes/api/internal/maintenance/+server.ts:36
lifecycle.trustpromotes organizations usingpromoteEstablished, whose eligibility is based onorgs.created_atrather than when the organization became paid. A long-lived free organization that upgrades to a paid plan is therefore promoted toestablishedon the next maintenance tick instead of after the documented 14 days on a paid plan, bypassing the intended scan-before-publish trust period.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/lib/server/services/lifecycle.ts around line 101:
`lifecycle.trust` promotes a long-lived free organization to `established` on the next maintenance tick after it upgrades to a paid plan, instead of waiting 14 days on the paid plan. `promoteEstablished` uses `orgs.created_at` for the cutoff, so it bypasses the intended verified-organization scan hold; base eligibility on the paid-plan transition time instead.
Also found in 2 other location(s):
- apps/web/src/lib/server/trust.ts:59 -- `promoteEstablished` compares `created_at` with the 14-day cutoff, so an old free org that upgrades today is promoted to `established` on the next sweep instead of after 14 days on a paid plan. This bypasses the intended verified-org scan-before-publish period immediately after upgrading.
- apps/web/src/routes/api/internal/maintenance/+server.ts:36 -- `lifecycle.trust` promotes organizations using `promoteEstablished`, whose eligibility is based on `orgs.created_at` rather than when the organization became paid. A long-lived free organization that upgrades to a paid plan is therefore promoted to `established` on the next maintenance tick instead of after the documented 14 days on a paid plan, bypassing the intended scan-before-publish trust period.
42a576b to
7828110
Compare
|
Addressed the confirmed follow-ups in this review update:
Earlier reviewed fixes already cover canceling held publication, current-version mutation/scan selection, durable scan/poll recovery, and failure-safe suspension. Intentional decisions:
Validation results are recorded in the PR description. No deployment or live-provider claims are implied. |
|
Too many files changed for review (111 files, 100 file limit). Bypass the limit by tagging |
Four rate limit bindings (RL_UPLOAD, RL_PUBLISH, RL_AUTH, RL_ANON) replace the KV counters in auth-guard.ts. Uploads and site sessions are keyed by org, device auth by client address, and anonymous content fetches past the edge cache by IP. The wrangler drift check compares the bindings and their limits across environments; route tests swap the bindings for fakes with a per-name denial switch. The AUTH_GUARD namespace stays for the slug and query embedding caches. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
trust-policy.ts decides what each level may do: new orgs stay private, verified and established ones may publish. Signing in with a verified email promotes new to verified; the maintenance tick promotes verified orgs on a paid plan for 14 days to established. Uploads that land public (including HTML, which is forced public), visibility changes, renames to .html, and site sessions and commits all pass through requirePublishAllowed, which answers 403 "Verify your email to share publicly" for a new org. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every version that becomes public gets a scan job. A verified org's publish (visibility change, rename to .html, site commit) is held with publish_pending until the scanner clears it; established orgs publish now and are scanned after. The Scanner runs three checks and records a scan_verdicts row for each: the object's sha256 against blocked_hashes, the first bytes against the declared type (mime-sniff.ts), and the outbound links in HTML through the Cloudflare URL Scanner (UrlReputation; a Null answers clean when URLSCAN_API_KEY is unset, `fake:<verdict>` selects a fake). Link scans are collected by re-sending the job with the scan ids. Clean publishes a held row and purges the edge cache; malicious quarantines the file, which content routes then 404, and writes a notification; suspicious stays held for review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`/report` on every content host takes a report (JSON or the one-form page at `/report?f=<id>`) for a file that host serves, rate limited by address; the address is stored hashed. The Admin service carries the kill switch: suspendOrg sets trust to suspended, drops the slug cache so the host answers 404 at once, and purges the host from the edge through the Cloudflare API when CF_API_TOKEN and CF_ZONE_ID are set (logged when not). API keys and sessions for a suspended org stop resolving with 401. restoreOrg reverses it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ADMIN_USER_IDS names the WorkOS users who may operate the platform; requireAdmin accepts only a browser session for one of them, never an API key. /admin on the dashboard origin lists open reports, held and quarantined files with their verdicts, dead-lettered jobs, and recent orgs with trust, plan, and usage, with row actions for resolving a report, marking a file clean or malicious, bumping trust, suspending and restoring an org, and blocking a hash. Every action goes through /api/admin/*, which the route tests exercise for non-admins and admins. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
docs/abuse.md: the zone checklist (WAF managed rules, bot fight mode, hotlink protection off, Netcraft feed, DMCA agent, abuse@ mailbox), the trust levels and what each may do, the scan pipeline check by check and what each verdict does to the row, the report endpoint, the kill switch step by step including the manual purge when the zone API is not configured, the rate limit bindings, and how to work /admin. The admin review list also surfaces live files the scanner flagged after publish until an operator rules on them, and admin verdict rows record who made the call. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2e6e1bc to
1d969eb
Compare
1 participant
Add trust-based publishing, content scans, abuse reports, operator moderation, suspension, and request rate limits. Verified organizations hold newly published bytes until scanning completes; established organizations use scanning after publication. Owner views show pending and quarantined states, and signed previews continue to work for held sites and older private versions.
Publication and moderation lock current metadata before deciding visibility. Historical versions require their own clearance for anonymous access. Scan obligations survive queue outages, stale results cannot replace newer decisions, and partial URL submissions retain successful scans. Reports have bounded request bodies; moderation checks the displayed version and works under the restricted database role. Cached host mappings recheck current organization trust. Device polling uses compatible slowdown responses and respects retry delays. HTML parsing handles effective base URLs; incomplete inspections retain a suspicious floor. File requests are limited before metadata lookup, held-site tabs open during the click, and site publication locks its current trust decision.
Important files:
services/files/,services/sites/, andcontent-version-access.ts: publication holds, version-specific access, and signed owner previews.services/scanner.ts,scan-jobs.ts, andhtml-links.ts: bounded inspection, durable recovery, verdict ownership, and HTML parsing, effective base URLs, and completeness limits.services/admin.tsandroutes/admin/+page.svelte: reports, version-aware moderation, and suspension controls.migrations-pg/0007_scans.sqlthrough0009_scan_recovery.sql: moderation records and persisted scan obligations.Validation: full root unit/shared/CLI/rune suites, route/Postgres integration tests, TypeScript/Effect/Svelte checks, formatting, and Worker build. Four broad review passes completed; the last pass had two accepted findings, both fixed and independently reviewed in a targeted closeout. Bot closeout covered 182 route/Postgres cases; two outdated verdict assertions were corrected and all 37 affected cases passed. HTML parser and MIME tests passed, including the bounded PE-signature follow-up; final targeted review was clean. Provider tests use mocks and local bindings; live Cloudflare URL scanning, cache purging, and deployed queue delivery remain separate verification steps.
Stack layer 8/11: depends on #36; followed by #38.
Final bot followup also recognizes every universal Mach-O header variant. All9 MIME tests passed; targeted independent review clean.
Note
Add content scanning, trust levels, rate limits, and admin abuse controls
new,verified,established,suspended) in trust-policy.ts and trust.ts that control publish eligibility, hourly publish quotas (30 verified / 300 established), and whether scans run before or after publicationAuthGuardwith aRateLimitsEffect service in rate-limits.ts backed by four Cloudflare Worker rate-limit bindings (upload, publish, auth, anonymous), keyed by organization ID or client IP; deletes auth-guard.ts/reportabuse-report form in report/+server.tspublish_pendinguntil their scan completes; quarantined files are excluded from content queries and disabled in the dashboard UI; suspended organizations are rejected at authentication time with a 403; theDashboardFileSchemain packages/shared/src/index.ts now requiresquarantinedandpublishPendingboolean fields, and theJobSchemascan variant accepts an optionalurlScanobject — out-of-tree consumers of these schemas must be updatedMacroscope summarized 1d969eb.