base/docs/ui-react-crud-security.md

Security Review — ui-react Collection + Record CRUD

Commit reviewed: f7f5ae77 (feat: ui-react collection + record CRUD) Date: 2026-04-12 Reviewer: internal security pass Scope: collections.$id.tsx, collections.$id.records.tsx, collections.$id.records.$recordId.tsx, __root.tsx

---

Findings Summary

#	Severity	Title	Status
1	HIGH	Realtime subscription thrash on query refetch	vulnerable
2	MEDIUM	JSON.parse in buildFormData throws unhandled, crashes entire submit	vulnerable
3	MEDIUM	Bulk delete captures collectionName by closure, not by value at confirm time	safe (re-evaluated)
4	LOW	No double-submit guard on record create (new record path)	vulnerable
5	LOW	Auth token expiry during long edit silently loses work	vulnerable
6	INFO	activeProps className replaces base class instead of merging	safe (cosmetic)
7	INFO	Rule fields are passthrough to server (no client-side escaping)	safe (by design)

---

Blue's Attack Vectors -- Detailed Analysis

Vector 1: Route parameter injection ($id path traversal)

Status: safe Severity: info Evidence: TanStack Router's $id param is extracted from a URL path segment. The browser's URL parser and TanStack's path matcher enforce that $id maps to a single path segment -- / characters cannot appear in the param value because the router splits on / before matching. A URL like /collections/..%2F..%2Fsettings/records would need $id = "../../settings" but the router sees four segments (collections, ..%2F..%2Fsettings, records, ...) and ..%2F..%2Fsettings is the literal param value. This value is then passed to base.collections.getOne(id) which calls encodeURIComponent(id) in the PocketBase SDK's baseCrudPath getter before constructing the API URL. The Go backend receives ..%252F..%252Fsettings -- no traversal.

Additionally, even if the param were decoded, the PocketBase Go backend resolves collections by name or 15-char ID lookup in the DB, not by filesystem path. There is no path traversal surface.

Vector 2: Stored XSS via record text fields in CellValue

Status: safe Severity: info Evidence: CellValue in collections.$id.records.tsx:360-382 renders all values through React JSX expressions ({String(val)}, {JSON.stringify(val).slice(0, 80)}, {val.join(', ')}). React auto-escapes all text content placed inside JSX expressions. There are no dangerouslySetInnerHTML calls, no <a href> rendering of URL fields (URL values are displayed as text via the default case {String(val)}), and no raw HTML injection path.

The editor field type (rich text) is also rendered as plain text in the table -- the CellValue default case treats it as a string. In the record editor form, editor fields use a <textarea> which naturally escapes HTML.

JSON fields use JSON.stringify(val).slice(0, 80) -- JSON.stringify escapes all special characters in strings, so no XSS through JSON display.

Vector 3: _new sentinel collision with real record ID

Status: safe Severity: info Evidence: collections.$id.records.$recordId.tsx:25 uses const isNew = recordId === '_new'. PocketBase record IDs are 15-character alphanumeric strings generated server-side (pattern: [a-z0-9]{15}). The string _new is 4 characters and starts with _, which cannot be generated by PocketBase's ID generator. The PocketBase SDK also rejects custom IDs that don't match its ID pattern on create. Zero collision risk.

Vector 4: Realtime subscription leak on rapid collection switching

Status: vulnerable Severity: HIGH

Evidence: collections.$id.records.tsx:50-73

The useEffect depends on collection.data (line 73: [collection.data, qc]). collection.data is an object reference from useQuery. Every time the query refetches -- which happens on every realtime event because the subscription callback calls qc.invalidateQueries({ queryKey: ['records', name] }) -- React Query returns a new object reference for collection.data (even if the data is structurally identical). This triggers the effect cleanup and re-run.

Attack scenario (concrete):

1. User navigates to a busy collection receiving ~5 writes/second.
2. Each write triggers the realtime callback, which invalidates the records query.
3. The records query refetch also invalidates the ['collections', id] query (due to query observer re-evaluation).
4. collection.data gets a new object reference, triggering the useEffect cleanup+re-run cycle.
5. Each cycle: unsubscribe old SSE connection, create new SSE connection. Under load, this creates a subscribe/unsubscribe storm that:
   • Opens/closes SSE connections rapidly (TCP overhead, server-side goroutine churn)
   • May hit the PocketBase server's max concurrent SSE connections
   • Causes visible UI flicker as records list goes loading->loaded repeatedly

Even without high write rates, simply navigating between collections rapidly creates orphaned subscriptions during the window between subscribe() call and .then() resolution, IF the user navigates away before the promise resolves AND navigates to another collection before React's cleanup cycle completes. The cancelled flag handles the promise resolution correctly, but the frequency of the subscribe/unsubscribe cycle is the issue.

Fix: Stabilize the effect dependency. Use collection.data?.name (a string primitive) instead of collection.data (an object reference).

Vector 5: Bulk delete targeting wrong collection after navigation

Status: safe Severity: N/A

Evidence: collections.$id.records.tsx:84-92

The bulkDel mutation captures collectionName from the component closure. The collectionName is derived from collection.data?.name ?? id which updates when id (from route params) changes. When the user navigates to a different collection, the entire component remounts because the route param $id changes, which means bulkSelected state resets to new Set() and collectionName is fresh. The mutation cannot fire against a stale collection because the selection is empty after navigation.

Confirmed safe. The React component identity is tied to the route, so navigation = remount = clean state.

Vector 6: FormData field pollution bypassing schema validation

Status: safe (client-side), info (server behavior) Severity: info

Evidence: buildFormData in collections.$id.records.$recordId.tsx:216-258 iterates over fields (the collection schema from the server) and only appends values for fields that exist in the schema. Unknown fields in the data object are silently dropped because only field.name keys from the schema are iterated. The loop for (const field of fields) is schema-driven, not data-driven.

However, the fileUploads ref (line 251-255) iterates over Object.entries(fileUploads.current) which is populated by handleFileChange from <input type="file"> elements. The file input's onChange handler is bound to a specific field.name, so only schema-valid field names can appear in fileUploads. An attacker with devtools could modify the DOM to add arbitrary file inputs, but the PocketBase server validates fields against the collection schema and rejects unknown field names with a 400 error.

Vector 7: Auth token expiry during long edit sessions

Status: vulnerable Severity: LOW

Evidence: The PocketBase SDK does NOT have autoRefresh enabled (it requires explicit opt-in via authWithPassword({autoRefresh: true})). The admin UI in lib/base.ts creates a bare new PocketBase(url) with no auto-refresh. The auth check in beforeLoad only runs on route transitions, not on API calls.

Attack scenario:

1. Admin opens record editor, starts editing a complex record.
2. Token expires (default PocketBase token lifetime: configurable, typically 14 days for superusers, but can be set shorter).
3. Admin clicks Save.
4. PocketBase SDK sends the request with the expired token.
5. Server returns 401.
6. save.error displays "Something went wrong" (the PocketBase SDK's generic error message for non-200 responses).
7. The edit is lost if the user navigates away to re-authenticate.

The save.error display on line 179 of $id.tsx and line 145 of $recordId.tsx will show the error, but there is no redirect to /login and no retry mechanism. The user must manually navigate to login, then back, and re-enter their changes.

For an admin tool with long token lifetimes, this is low severity but poor UX.

Vector 8: File upload MIME type bypass

Status: safe (server-side defense) Severity: info

Evidence: The client-side file input has no accept attribute restriction (line 404 of $recordId.tsx), so any file type can be uploaded. However, the PocketBase Go backend:

1. Detects content type from file magic bytes, not the Content-Type header (tools/filesystem/filesystem.go:444 uses br.ContentType() which reads the actual blob content type).
2. Only serves images/video/audio/PDF inline; everything else gets Content-Disposition: attachment (inlineServeContentTypes allowlist at filesystem.go:400-410).
3. Sets Content-Security-Policy: default-src 'none'; media-src 'self'; style-src 'unsafe-inline'; sandbox on every file download (filesystem.go:458), which blocks script execution even for inline-served files.
4. HTML files uploaded with a .png extension would be detected as text/html by magic bytes and served as attachment, not inline.

The backend defense is solid. An HTML file disguised as an image cannot execute JavaScript when served.

Vector 9: JSON field injection via __proto__

Status: safe (with caveat) Severity: info

Evidence: buildFormData at line 230-233 of $recordId.tsx:

if (field.type === 'json' && typeof val === 'string' && val.trim()) {
    JSON.parse(val);  // validation only -- return value discarded
    fd.append(field.name, val);
}

The JSON.parse(val) validates syntax but the return value is discarded. The original string val containing {"__proto__": {"polluted": true}} is sent as-is to the server. However:

1. The Go backend's encoding/json unmarshaler does not have prototype pollution -- Go structs are statically typed. __proto__ is just a regular JSON key with no special semantics.
2. The PocketBase JSON field stores the value as a raw JSON blob in SQLite/Postgres. No deserialization into a prototype chain occurs.
3. On read-back, the value is served as-is and the frontend JSON.stringify in CellValue just displays it.

No prototype pollution path exists because the server is Go (not JavaScript). The client never deserializes untrusted JSON into live objects -- it treats JSON field values as strings.

Caveat: The JSON.parse call (line 232) throws on invalid JSON. This exception is NOT caught in buildFormData. It propagates up to the mutationFn which makes save.mutate() reject. The save.error state then displays the SyntaxError message. This is functional but the error message will be something like "SyntaxError: Unexpected token x in JSON at position 5" which is not user-friendly.

Vector 10: Collection rule field accepting executable content

Status: safe (by design) Severity: info

Evidence: Rule fields in collections.$id.tsx:330-345 are plain <input> elements registered with react-hook-form. The values are sent as strings to the PocketBase server, which parses them using its own filter/rule parser. The server:

1. Validates the rule syntax against its own grammar (not eval).
2. Returns a 400 error for invalid rule expressions.
3. Does not execute rules as JavaScript or SQL -- rules are parsed into an AST and evaluated against the request context.

The client is correctly a passthrough. Attempting to validate or escape rules on the client would be redundant and could break valid rule expressions. The server is the authority.

---

Additional Findings (Not on Blue's List)

Finding A: confirm() usage for destructive operations

Status: safe Evidence: All destructive operations use window.confirm():

• Collection delete: $id.tsx:119
• Single record delete: $id.records.tsx:141
• Bulk record delete: $id.records.tsx:135
• Record editor delete: $recordId.tsx:96

For an internal admin tool, window.confirm() is acceptable. It blocks the UI thread and requires explicit user interaction.

Finding B: Password in plain JSON over the wire

Status: safe (channel secured) Evidence: Password is sent via FormData.append('password', pw) at $recordId.tsx:245. The PocketBase admin UI is served from the same origin as the API server. The Go server at apis/serve.go:115 supports HTTPS configuration. The CSP at line 104 allows connect-src 'self', constraining API calls to the same origin. When deployed with HTTPS (which is required for production per the admin UI's same-origin design), passwords are encrypted in transit.

No plaintext password storage occurs -- PocketBase server hashes passwords with bcrypt server-side before storage.

Finding C: activeProps className behavior

Status: safe (cosmetic only) Evidence: In __root.tsx:20-24, activeProps replaces the className entirely rather than merging with the base className. This means an active link loses hover:bg-neutral-800 (which is fine since active state already has bg-neutral-800). The Dashboard link uses activeOptions={{ exact: true }} (line 23) to prevent false activation on child routes. The Collections link does NOT use exact: true, which means it stays highlighted when viewing /collections/abc123 -- this is correct and intended behavior for a parent nav item.

Finding D: Double-submit on record create

Status: vulnerable Severity: LOW

Evidence: $recordId.tsx:136-137:

disabled={(!isNew && !form.formState.isDirty) || save.isPending}

For isNew === true, the disabled condition simplifies to save.isPending. This means:

1. First click: save.isPending is false, button is enabled, mutation fires.
2. save.isPending becomes true, button is disabled.
3. BUT: there is a React render cycle between the click event and the isPending state update. A sufficiently fast double-click can fire two mutations before the first render cycle completes.

This is a theoretical race in React's batched state updates. In practice, React 19's automatic batching should prevent this in most cases. The PocketBase server does not have idempotency keys, so two creates would produce two records.

Practical risk: Very low. React 19 batches state updates synchronously within event handlers. The mutation's isPending state is set synchronously by TanStack Query before the handler returns. The window is effectively zero for human interaction. Downgrading from medium to low.

Finding E: beforeLoad auth check -- all protected routes covered

Status: safe Evidence: Auth gate audit:

Route	beforeLoad present	Notes
/ (index)	YES	index.tsx:54
/collections	YES	collections.tsx:86
/collections/$id	YES	$id.tsx:404
/collections/$id/records	YES	$id.records.tsx:389
/collections/$id/records/$recordId	YES	$recordId.tsx:481
/settings	YES	settings.tsx:51
/settings/ (index)	YES	settings.index.tsx:4 (redirects to /settings/smtp)
/settings/smtp	inherited	Parent /settings has beforeLoad
/settings/auth	inherited	Parent /settings has beforeLoad
/settings/superusers	inherited	Parent /settings has beforeLoad
/settings/* (all)	inherited	Parent /settings has beforeLoad
/logs	YES	logs.tsx:73
/login	NO (correct)	Public route

TanStack Router's beforeLoad runs top-down: parent before children. Since /settings has beforeLoad, all /settings/* child routes are protected even without their own beforeLoad. This is documented TanStack Router behavior and confirmed in the route tree.

All routes are correctly protected.

Finding F: Subscription thrash (DETAILED) -- the HIGH finding

Status: vulnerable Severity: HIGH

Location: collections.$id.records.tsx:50-73

Root cause: The useEffect at line 50 has [collection.data, qc] as its dependency array. collection.data is a React Query cache object. React Query returns a NEW object reference on every refetch, even when the data is structurally identical (it uses structural sharing by default, but the top-level reference still changes). This means:

1. Realtime event fires.
2. Subscription callback invalidates ['records', name].
3. React Query refetches records.
4. The ['collections', id] query may also be re-evaluated (React Query's observer).
5. collection.data gets a new reference.
6. React re-runs the effect: cleanup (unsubscribe) + setup (new subscribe).
7. New subscription immediately receives pending events, goto step 1.

Concrete reproduction:

1. Open /collections/posts/records on a collection with active writes.
2. Open browser DevTools > Network tab.
3. Watch SSE connections: you'll see repeated CONNECT/DISCONNECT cycles on
   /v1/realtime as the effect tears down and rebuilds the subscription.
4. Each cycle is a full SSE handshake (HTTP upgrade, auth token validation,
   server-side subscription map update).

Impact: Server resource exhaustion under moderate write load. Each subscribe/unsubscribe cycle costs a TCP connection setup, Go goroutine allocation on the server, and SSE event replay. With 10 concurrent admin sessions on a busy collection, this can saturate the SSE connection pool.

Fix applied: Change the dependency from collection.data to collection.data?.name (a string primitive that doesn't change reference on refetch).

---

Fixes Applied

Fix 1: Stabilize realtime subscription dependency (HIGH)

File: ui-react/src/routes/collections.$id.records.tsx

Change the useEffect dependency from collection.data to collection.data?.name:

-    }, [collection.data, qc]);
+    }, [collection.data?.name, qc]);

The effect only needs to re-run when the collection name changes (i.e., when navigating to a different collection). The collection.data object changing on refetch is irrelevant to the subscription, which is keyed by name.

Fix 2: Wrap JSON.parse in try/catch (MEDIUM)

File: ui-react/src/routes/collections.$id.records.$recordId.tsx

The unhandled JSON.parse throw produces a poor error UX. Wrap it to produce a clear validation error:

         if (field.type === 'json' && typeof val === 'string' && val.trim()) {
-            // Validate JSON before sending
-            JSON.parse(val);
-            fd.append(field.name, val);
+            try {
+                JSON.parse(val);
+            } catch {
+                throw new Error(`Invalid JSON in field "${field.name}"`);
+            }
+            fd.append(field.name, val);

---

Blue Handoff

What Blue got right:

• The cancelled flag pattern for async subscription cleanup is correct and handles the race between cleanup and promise resolution. Solid async hygiene.
• beforeLoad auth gates on every protected route, including the parent /settings covering all children. No auth bypass.
• CellValue rendering is safe -- no dangerouslySetInnerHTML, no <a href> with user-controlled URLs. React's auto-escape is correctly relied upon.
• buildFormData is schema-driven (iterates fields, not data), preventing field pollution.
• The _new sentinel is safe against collision with PocketBase's ID format.
• confirm() on all destructive operations.
• Disabled submit button during mutation pending state.

What Blue missed:

• The useEffect dependency on collection.data (object reference) instead of collection.data?.name (stable string) causes subscription thrash under any write load.
• The unhandled JSON.parse throw in buildFormData produces a cryptic SyntaxError in the UI instead of a user-friendly validation message.

Fix priority for Blue:

1. Subscription dependency stabilization (HIGH -- causes real server load under normal usage)
2. JSON.parse error handling (MEDIUM -- poor UX but no security impact)
3. Consider autoRefresh for long-lived admin sessions (LOW -- optional UX improvement)

Re-review scope: Only the subscription fix needs verification -- confirm that the subscription correctly re-establishes when navigating between collections (the collection.data?.name dep should change on navigation since id changes).