Skip to content

fix(file-viewer): sanitize docx hyperlink hrefs to block javascript: XSS#5599

Merged
waleedlatif1 merged 1 commit into
stagingfrom
worktree-fix+docx-xss-hyperlink
Jul 11, 2026
Merged

fix(file-viewer): sanitize docx hyperlink hrefs to block javascript: XSS#5599
waleedlatif1 merged 1 commit into
stagingfrom
worktree-fix+docx-xss-hyperlink

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • docx-preview copies external hyperlink targets straight into anchor href with no scheme check, so a crafted .docx could render a javascript: link into the app origin
  • Added sanitizeRenderedHyperlinks (reusing the same http/https/mailto allowlist already used by the PPTX renderer) and run it on the rendered DOM right after renderAsync completes
  • Same-document bookmark anchors (#section) are left intact; surviving external links get rel="noopener noreferrer"
  • Moved the shared isAllowedExternalUrl allowlist from lib/pptx-renderer/utils to lib/core/security since it's now used by both renderers
  • Fixes both the workspace file viewer and the public /f/[token] share page, which reuse the same component

Type of Change

  • Bug fix (security)

Testing

  • Added unit tests for sanitizeRenderedHyperlinks covering javascript:, data:, vbscript: stripping, bookmark preservation, and allowed-link passthrough
  • Existing PPTX url-safety tests still pass after the move

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)

Sanitize anchor hrefs rendered by docx-preview after render, stripping
any scheme outside http/https/mailto (same allowlist already used by
the PPTX renderer). Covers both the workspace file viewer and the
unauthenticated public share page, which reuse the same component.
@vercel

vercel Bot commented Jul 11, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 11, 2026 6:26pm

Request Review

@cursor

cursor Bot commented Jul 11, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Targeted security hardening on untrusted document previews with unit tests; PPTX behavior is unchanged aside from import path.

Overview
Security fix for DOCX previews: After docx-preview renders a document, the preview DOM is passed through sanitizeRenderedHyperlinks, which strips dangerous href values (javascript:, data:, vbscript:, relative URLs) while keeping # bookmarks and allowed http/https/mailto links (with rel="noopener noreferrer").

Shared URL policy: isAllowedExternalUrl moves from lib/pptx-renderer/utils to lib/core/security/url-safety; PPTX render paths import from there. The old PPTX-only module and its tests are removed in favor of centralized tests (including new sanitizeRenderedHyperlinks coverage).

Reviewed by Cursor Bugbot for commit 4c70118. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds shared URL safety handling for rendered document links. The main changes are:

  • Moves the PPTX URL allowlist into lib/core/security.
  • Sanitizes DOCX-rendered anchor href values after docx-preview finishes rendering.
  • Preserves same-document bookmarks and adds rel protection to allowed external links.
  • Moves and expands URL safety tests for the shared helper.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The DOCX sanitizer runs after rendering and before later styling work.
  • The PPTX allowlist behavior is preserved after the import move.

Important Files Changed

Filename Overview
apps/sim/lib/core/security/url-safety.ts Adds the shared URL allowlist and DOCX anchor sanitizer.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/docx-preview.tsx Runs hyperlink sanitization after DOCX rendering completes.
apps/sim/lib/pptx-renderer/core/viewer.ts Updates the PPTX viewer to import the shared URL allowlist.
apps/sim/lib/pptx-renderer/renderer/shape-renderer.ts Updates the shape renderer to import the shared URL allowlist.
apps/sim/lib/pptx-renderer/renderer/text-renderer.ts Updates the text renderer to import the shared URL allowlist.
apps/sim/lib/core/security/url-safety.test.ts Moves the allowlist tests and adds coverage for DOCX link sanitization.

Reviews (1): Last reviewed commit: "fix(file-viewer): sanitize docx hyperlin..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 merged commit b731162 into staging Jul 11, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the worktree-fix+docx-xss-hyperlink branch July 11, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant