Skip to content

fix(security): close CodeQL alerts for URL sanitization and workflow permissions - #55

Merged
dryor merged 1 commit into
mainfrom
fix/codeql-security-alerts
Aug 11, 2026
Merged

fix(security): close CodeQL alerts for URL sanitization and workflow permissions#55
dryor merged 1 commit into
mainfrom
fix/codeql-security-alerts

Conversation

@dryor

@dryor dryor commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Closes CodeQL alert Add ADR for figma-scraper-core architecture #2 (js/incomplete-url-substring-sanitization, high): parseFigmaUrl() in packages/core/src/figma/core.ts used hostname.endsWith("figma.com"), which a host like evil-figma.com would satisfy without actually being Figma's domain. Replaced with an exact host match plus a proper .figma.com subdomain suffix check.
  • Closes CodeQL alert Add Figma integration specification tests #1 (actions/missing-workflow-permissions, medium): .github/workflows/test.yml ran on pull_request (including forks) without a permissions block, so its GITHUB_TOKEN got the repo's default (potentially broader) permissions. Added permissions: contents: read at the workflow level — the job only checks out, installs, builds, and tests.

Test plan

  • pnpm --filter @figtools/core test — 51/51 tests pass, including a new test covering the evil-figma.com bypass case
  • pnpm --filter @figtools/core build — compiles cleanly
  • Confirm both alerts move to "Closed" in Security → Code scanning after this merges to main

🤖 Generated with Claude Code

…permissions

The figma.com host check used endsWith(), which a host like
evil-figma.com would satisfy without being Figma's domain. The
Quality Gate workflow also ran PRs (including from forks) with the
default GITHUB_TOKEN permissions instead of a scoped-down set.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dryor
dryor merged commit bf332c5 into main Aug 11, 2026
4 checks passed
dryor added a commit that referenced this pull request Aug 11, 2026
Merging main (PR #55) brought in a test added after this branch was
cut, still calling the pre-rename resolveUrl.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dryor added a commit that referenced this pull request Aug 11, 2026
PR #55 (security fix) merged to main without a changeset, so its
patch bump was never queued. Adding it here alongside this PR's own
changeset since this branch already has main merged in.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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