fix(security): harden glob matching, archive parsing, redaction and media handling - #6030
fix(security): harden glob matching, archive parsing, redaction and media handling#6030waleedlatif1 wants to merge 1 commit into
Conversation
…edia handling
Glob matching (copilot VFS):
- match globs with RE2 instead of micromatch's backtracking engine, and
translate picomatch's output rather than trusting it: reject escape
passthrough (\A, \z, \p{L}) that RE2 reads as anchors and Unicode
classes, and class ranges straddling the private-use segment markers
Archive parsing:
- scan the whole buffer for EOCD records, matching JSZip and SheetJS
rather than the spec's trailing window
- treat a resolvable-but-empty central directory as unverifiable
- bound central-directory scanning with a shared record budget
- keep stray EOCD byte sequences in non-ZIP documents a no-op so the
OLE2 and plaintext fallbacks still run
Redaction:
- fix quadratic acronym-boundary backtracking (73s to 1ms at 400k chars)
- catch plural and value-suffixed secret keys, and one-shot locator URLs
Other:
- guard YAML alias expansion in the JSON/YAML chunker
- pin fal.ai queue polling to its origin and cap the response body
- escape ffmpeg drawtext values through a file instead of the filtergraph
- resolve redirects against the registrable domain and drop credential
headers across sites
- let the file preview error boundary retry instead of latching
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Copilot VFS glob no longer uses micromatch’s backtracking Zip/OOXML guards scan the full buffer for EOCD (not only a trailing window), handle prepended bytes, decoy EOCDs, count mismatches, ZIP64 declared sizes, and bounded walk budgets—rejecting forged expansion before JSZip runs. Redaction reworks key normalization (fixes quadratic acronym splitting), expands sensitive-key rules (plural/value/locator keys, credential literals in strings,
Video tool API downloads provider URLs and polls Fal.ai queue endpoints through DNS-validated, IP-pinned fetches with response size caps; Fal queue URLs are origin-resolved/stripped ( File viewer: PptxPreview is lazy-loaded; Documentation comment added for SheetJS CDN pin on OneDrive upload and xlsx parser. Reviewed by Cursor Bugbot for commit 34ff739. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 34ff739. Configure here.
| return ( | ||
| <PreviewErrorBoundary key={file.id} label='PowerPoint'> | ||
| <PptxPreview file={file} workspaceId={workspaceId} /> | ||
| </PreviewErrorBoundary> |
There was a problem hiding this comment.
PPTX boundary ignores content version
Medium Severity
The PowerPoint preview wraps PreviewErrorBoundary with key={file.id} only, while the PDF viewer keys its boundary with file.id and preview.dataUpdatedAt. After a preview crash, updating the same file’s binary without changing file.id leaves the boundary in its error state even though PptxPreview reloads internally, so users stay on the failure UI until they click Try again.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 34ff739. Configure here.
Greptile SummarySecurity hardening across glob matching, archive inspection, redaction, media tooling, and SSRF-sensitive fetches, with no dependency changes.
Confidence Score: 5/5This PR appears safe to merge; the hardening paths are fail-closed or origin-pinned and are covered by discriminating tests. Investigated glob RE2 translation, zip EOCD/CD fail-closed behavior, redaction user-file filtering, fal queue URL pinning, redirect credential stripping, ffmpeg drawtext isolation, loop condition serialization, and YAML alias guards; no concrete reachable defect remained after checking those contracts against callers and tests.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/copilot/vfs/operations.ts | Replaces micromatch.isMatch with RE2-backed compileGlobMatcher, segment markers for dot/newline semantics, and safety caps. |
| apps/sim/lib/file-parsers/zip-guard.ts | Full-buffer EOCD scan, multi-candidate worst-case limits, unverifiable CD fail-closed, shared walk budget. |
| apps/sim/lib/core/security/redaction.ts | Key normalization, broader secret/locator matching, credential containers, user-file display filtering, value-literal redaction. |
| apps/sim/lib/core/security/input-validation.server.ts | Cross-site redirect credential stripping via tldts registrable domain; stripAuth still drops Authorization on same-site hops. |
| apps/sim/lib/media/falai.ts | Origin-pinned queue URL resolution, /response strip, app-id polling path, capped JSON polls via secure fetch. |
| apps/sim/app/api/tools/video/route.ts | Provider asset downloads and Fal polls go through SSRF-guarded clients; Veo API key gated on genuine Google hosts. |
| apps/sim/lib/media/ffmpeg.ts | Overlay text written to temp textfile with double-escaped path; expansion disabled. |
| apps/sim/executor/orchestrators/loop.ts | While-condition string interpolation uses JSON.stringify to prevent quote/newline breakout into the VM. |
| apps/sim/lib/chunkers/json-yaml-chunker.ts | YAML load path runs assertYamlWithinLimits; complexity errors rethrow instead of text fallback. |
| apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-shared.tsx | PreviewErrorBoundary retries in place or offers reload for chunk-load failures; PptxPreview lazy-loaded behind boundary. |
Reviews (1): Last reviewed commit: "fix(security): harden glob matching, arc..." | Re-trigger Greptile
1 participant
Summary
Security hardening split out of #6013 so it can be reviewed and merged on test evidence alone. No dependency changes — every fix stands on staging's existing
re2jsandtldts.\A/\z/\p{L}through verbatim and RE2 reads those as anchors and Unicode classes, which made\A**/**select every path and let\p{Any}consume the segment markers that encode thedot:falseguarantee. Class ranges straddling the marker block are rejected for the same reason.Type of Change
Testing
2,837 tests pass across 162 files;
tsc, biome andcheck:api-validationclean.Every security fix is pinned by a test proven to fail against the pre-fix code — the guards were verified by constructing working exploits, not by inspection. Two vacuous tests found during that process (one passed with the guard call deleted) were rewritten to discriminate.
Checklist