fix(security): harden findings — path traversal, SSRF, IDOR, file auth, credential access by waleedlatif1 · Pull Request #4571 · simstudioai/sim

waleedlatif1 · 2026-05-12T21:41:32Z

Summary

Supabase path traversal: validate `table` param with strict identifier regex + `encodeURIComponent` in get_row, delete, update tools; add missing empty-filter guard to update (matching delete.ts)
Missing `verifyFileAccess`: add ownership check before `downloadFileFromStorage` in SFTP, SMTP, and SharePoint upload routes (matching WordPress reference pattern)
Credential authorization: replace bare `resolveOAuthAccountId` + workspace-only check with `authorizeCredentialUse` in Gmail labels, OneDrive folders, and Wealthbox items routes — enforces `credentialMember` table and fixes token refresh using requester's userId instead of account owner's
A2A DNS rebinding SSRF: thread pre-resolved IP from `validateUrlWithDNS` into A2A SDK via pinned fetch for `JsonRpcTransportFactory`, `RestTransportFactory`, and `DefaultAgentCardResolver`, closing the TOCTOU window
SSH OOM: cap stdout/stderr accumulation at 16 MB with truncation marker in `executeSSHCommand`
Form soft delete: replace `db.delete()` with `db.update({ archivedAt })` matching schema intent
Workflow import variables: fix `Array.isArray()` guard that silently dropped all variables (export format is Record, not Array)
Storage quota bypass: apply `checkStorageQuota` and file size check to `mothership` upload context
Workspace env race: replace read-modify-write upsert with atomic JSONB `||` merge (PUT) and `-` key removal (DELETE)

Type of Change

Bug fix

Testing

Tested manually

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…rfaces - Supabase tools (get_row, delete, update): validate table name with strict identifier regex and encodeURIComponent to prevent LLM-controlled path traversal to admin endpoints; add missing empty-filter guard to update matching the delete.ts pattern - SFTP/SMTP/SharePoint upload routes: add verifyFileAccess ownership check before downloadFileFromStorage, matching the WordPress reference pattern; rejects files the requesting user does not own with 404 - Gmail labels, OneDrive folders, Wealthbox items (×2): replace bare resolveOAuthAccountId + workspace-only membership check with authorizeCredentialUse which enforces credentialMember table; use credentialOwnerUserId for token refresh instead of bare accountRow.userId - A2A utils: thread pre-resolved IP from validateUrlWithDNS into A2A SDK via pinnedFetch (secureFetchWithPinnedIP) for JsonRpcTransportFactory, RestTransportFactory, and DefaultAgentCardResolver, closing the TOCTOU DNS rebinding window - SSH utils: cap stdout/stderr accumulation at 16 MB with truncation marker to prevent OOM from unbounded command output - Form DELETE route: replace db.delete() with db.update({archivedAt}) for true soft delete matching the schema's archivedAt column - Workflow admin import: fix Array.isArray() guard that silently dropped all variables (export format is Record, not Array) - Multipart upload: apply checkStorageQuota and MAX_WORKSPACE_FILE_SIZE to mothership context, closing the quota bypass for workspace-scoped storage

…SONB ops PUT: use `variables || excluded.variables` in onConflictDoUpdate so concurrent writes merge atomically in the DB instead of last-writer-wins at the application layer. DELETE: replace the read-modify-write upsert with a single UPDATE that removes keys via the JSONB `-` operator, preventing concurrent deletes from resurrecting previously-removed secrets.

vercel · 2026-05-12T21:41:38Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		May 13, 2026 0:24am

cursor · 2026-05-12T21:41:42Z

PR Summary

Medium Risk
Touches security-sensitive surfaces (credential authorization, file-download authorization, SSRF protections) and changes network/stream handling; mistakes could cause unintended access denials or regress integrations.

Overview
Security hardening across tool + OAuth routes. Replaces ad-hoc session/workspace checks with centralized authorizeCredentialUse in Gmail/OneDrive/Wealthbox routes (including service-account handling) and tightens credentialId validation.

File download IDOR protections. Adds assertToolFileAccess and applies it before downloadFileFromStorage in SFTP/SMTP/SharePoint/WordPress tool routes; multipart uploads now enforce workspace file limits/quota for mothership as well as workspace.

SSRF/DNS rebinding + DoS mitigations. A2A client creation now pins outbound fetches to the DNS-resolved IP via custom A2A transports/card resolver; secureFetchWithPinnedIP now streams response bodies while enforcing max-size limits; SSH command execution caps stdout/stderr accumulation at 16MB with truncation markers.

Correctness tweaks. Form deletion is now a true soft-delete (archivedAt/isActive=false), admin workflow import correctly ingests variables supplied as an object, Supabase tools validate/encode table identifiers and update now requires a filter, and blockquote styling is normalized in several markdown renderers.

^{Reviewed by Cursor Bugbot for commit 31e97df. Configure here.}

greptile-apps · 2026-05-12T21:47:39Z

Greptile Summary

This PR is a broad security hardening pass covering nine distinct vulnerability classes across the Supabase tools, file-upload routes, credential/OAuth routes, the A2A transport layer, SSH command execution, workspace environment storage, and the admin workflow-import endpoint.

Supabase path traversal: validateDatabaseIdentifier + encodeURIComponent applied to params.table across all eight Supabase tools; update.ts now requires a non-empty filter; text_search.ts still leaves params.column and params.language unvalidated/unencoded in the URL (see inline comment).
File ownership (IDOR): assertToolFileAccess helper centralises the verifyFileAccess check and is wired into SFTP, SMTP, and SharePoint upload routes.
Credential authorization: Gmail labels, OneDrive folders, Wealthbox OAuth, and Wealthbox items routes all migrate to authorizeCredentialUse; CredentialAccessResult gains credentialType to eliminate a redundant DB round-trip in the Gmail route.
A2A SSRF (DNS rebinding), SSH OOM, workspace env race (pg_advisory_xact_lock), storage quota bypass, and workflow variable import fixes all look correct.

Confidence Score: 4/5

Safe to merge with one fix: the params.column injection gap in text_search.ts should be addressed before this ships.

The rest of the hardening work is thorough and well-implemented — file ownership, credential authorization, advisory locking, A2A SSRF, and SSH OOM are all correctly addressed. The only open defect is that text_search.ts validates the table name but leaves the column name and language parameter raw in the PostgREST URL, which is the same class of injection this PR is specifically closing for table names.

apps/sim/tools/supabase/text_search.ts — params.column and params.language need the same validateDatabaseIdentifier + encodeURIComponent treatment applied to params.table in the other Supabase tools.

Security Review

Query-parameter injection (apps/sim/tools/supabase/text_search.ts, line 96): params.column and params.language are interpolated directly into the PostgREST URL without identifier validation or URL-encoding. An attacker-controlled column name containing & or = can inject arbitrary query parameters; a language value containing ) breaks the operator syntax. All other Supabase tools guard params.table with validateDatabaseIdentifier but text_search.ts is incomplete.

Important Files Changed

Filename	Overview
apps/sim/tools/supabase/text_search.ts	Adds table-name validation but leaves params.column and params.language unvalidated/unencoded in the PostgREST URL, enabling query-parameter injection.
apps/sim/lib/a2a/utils.ts	Threads the pre-resolved IP from validateUrlWithDNS into a custom pinnedFetch that replaces the A2A SDK default fetch, closing the TOCTOU DNS-rebinding window.
apps/sim/lib/core/security/input-validation.server.ts	Refactors secureFetchWithPinnedIP from buffer-accumulation to a streaming ReadableStream body; adds memoised readBodyAsBuffer so json() then text() fallback patterns work.
apps/sim/app/api/workspaces/[id]/environment/route.ts	Replaces read-modify-write upsert with a transaction-scoped pg_advisory_xact_lock that serialises all writes for a workspace, correctly closing the concurrent first-insert race.
apps/sim/app/api/tools/smtp/send/route.ts	Replaces Promise.all parallel attachment downloads with a sequential loop that checks assertToolFileAccess per file before downloading.
apps/sim/app/api/files/multipart/route.ts	Extends the file-size cap and checkStorageQuota enforcement to the mothership upload context, closing a storage quota bypass.
apps/sim/lib/auth/credential-access.ts	Adds credentialType field to CredentialAccessResult and populates it across all four success paths in authorizeCredentialUse.
apps/sim/tools/supabase/update.ts	Adds table-name validation and throws on empty filter to prevent accidental full-table updates, matching the delete.ts safety guard.

Comments Outside Diff (1)

apps/sim/tools/supabase/text_search.ts, line 78-96 (link)

params.column is still interpolated directly as a query-parameter key with no identifier validation or encoding. A value like col&admin=true breaks the PostgREST URL structure and injects arbitrary extra query parameters — the exact class of injection this PR closes for table names. Additionally, language is embedded inside the operator parentheses without validation; a value containing ) or & would break the operator syntax and allow further parameter injection.

_{Reviews (11): Last reviewed commit: "fix(security): cleanup abort listener wh..." | Re-trigger Greptile}

- SMTP send: restructure attachment loop from Promise.all to sequential for...of so verifyFileAccess denial returns 404 instead of propagating as a generic 500 via the SMTP error classifier - Supabase tools: extend table-name validation and encodeURIComponent to the five previously missed tools — insert, upsert, count, query, text_search — completing coverage across all nine Supabase tools - Credential routes: remove unnecessary `request as any` casts in Gmail, OneDrive, and Wealthbox routes; authorizeCredentialUse already accepts NextRequest directly - Form soft delete: also set isActive=false alongside archivedAt so that any future code paths querying by isActive see a consistent state - SSH utils: fix exit code fallback from 0 to -1 so an abnormally closed connection that supplies no exit code is not reported as success - Workspace env: capitalize EXCLUDED.variables in the onConflictDoUpdate set clause to make the pseudo-table reference unambiguous

- fix(env): replace jsonb operators with transaction+FOR UPDATE read-modify-write - PUT: uses db.transaction + SELECT FOR UPDATE + JS merge to avoid lost-update race - DELETE: same pattern; fixes variable scope bug where current was referenced outside tx - removes broken || and - jsonb operators that fail on json-typed column - fix(ssh): trim truncated output consistently with non-truncated path - fix(gmail): remove redundant resolveOAuthAccountId call - adds credentialType field to CredentialAccessResult - authorizeCredentialUse now returns credentialType in all success paths - gmail/labels route uses authz.credentialType and authz.resolvedCredentialId directly - fix(supabase): centralize table identifier validation - adds validateDatabaseIdentifier() to input-validation.ts - all 8 supabase tools use the shared util instead of inline regex

…route The intermediate Record cast used 'string' for the type field which TypeScript correctly rejected — WorkflowVariable.type is 'VariableType', not string. Changed the cast to use VariableType so both branches typecheck correctly.

waleedlatif1 · 2026-05-12T22:21:37Z

@greptile

waleedlatif1 · 2026-05-12T22:21:41Z

@cursor review

waleedlatif1 · 2026-05-12T22:32:19Z

@greptile

waleedlatif1 · 2026-05-12T22:32:22Z

@cursor review

…ership branch

waleedlatif1 · 2026-05-12T22:47:22Z

@cursor review

waleedlatif1 · 2026-05-12T22:47:23Z

@greptile

…dyInit types in pinnedFetch

waleedlatif1 · 2026-05-12T23:37:25Z

@cursor review

waleedlatif1 · 2026-05-12T23:37:25Z

@greptile

…FileAccess in WordPress, memoize body buffer to prevent silent empty reads, fix ArrayBuffer type cast

waleedlatif1 · 2026-05-12T23:46:29Z

@cursor review

waleedlatif1 · 2026-05-12T23:46:30Z

@greptile

waleedlatif1 · 2026-05-12T23:58:10Z

@cursor review

waleedlatif1 · 2026-05-12T23:58:11Z

@greptile

… up in stream end/error/cancel

waleedlatif1 · 2026-05-13T00:11:43Z

@cursor review

waleedlatif1 · 2026-05-13T00:11:44Z

@greptile

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 0073513. Configure here.}

…exceeded

waleedlatif1 · 2026-05-13T00:25:19Z

@greptile

waleedlatif1 · 2026-05-13T00:25:22Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 31e97df. Configure here.}

waleedlatif1 added 2 commits May 12, 2026 14:39

waleedlatif1 changed the title ~~fix(security): harden HIGH deepsec findings — path traversal, SSRF, IDOR, file auth, credential access~~ fix(security): harden deepsec findings — path traversal, SSRF, IDOR, file auth, credential access May 12, 2026

waleedlatif1 changed the title ~~fix(security): harden deepsec findings — path traversal, SSRF, IDOR, file auth, credential access~~ fix(security): harden findings — path traversal, SSRF, IDOR, file auth, credential access May 12, 2026

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread apps/sim/app/api/tools/gmail/labels/route.ts Outdated

Comment thread apps/sim/app/api/tools/ssh/utils.ts