Skip to content

fix: address code scanning alerts#305

Merged
hqhq1025 merged 1 commit into
mainfrom
codex/security-code-scanning-fixes
May 4, 2026
Merged

fix: address code scanning alerts#305
hqhq1025 merged 1 commit into
mainfrom
codex/security-code-scanning-fixes

Conversation

@hqhq1025
Copy link
Copy Markdown
Collaborator

@hqhq1025 hqhq1025 commented May 4, 2026

Summary

  • Replace exporter HTML text extraction/entity decoding paths that triggered CodeQL with shared scanner helpers, preserving literal comparison text like 2 < 3 while still stripping real tags.
  • Add regression tests for Markdown and editable PPTX export text extraction.
  • Pin GitHub Actions to commit SHAs and restrict default workflow token permissions.
  • Harden pull_request_target PR review automation so fork PRs require the safe-to-review label before the write-token bot runs.
  • Remove unnecessary release friction: Snap remains best-effort and no longer gates provenance/publish.
  • Clarify AGENTS/CLAUDE license policy so shipped/runtime dependencies stay permissive while workflow-only tools can use copyleft licenses when not bundled or distributed.
  • Restore winget release automation and explicitly allow its workflow-only action in Dependency Review.

Why

GitHub code scanning currently reports CodeQL high alerts in exporter text cleanup and Scorecard alerts around workflow token permissions, unpinned actions, and pull_request_target risk. The exporter issue came from regex-based tag/entity handling; the workflow issues came from broad defaults and floating action tags.

The previous blanket AGPL/GPL rule was too broad for CI-only tooling. This PR keeps the product/distribution boundary strict while allowing isolated release automation that does not ship in the app.

Validation

  • pnpm --filter @open-codesign/exporters exec vitest run src/pptx.test.ts src/markdown.test.ts
  • pnpm typecheck
  • pnpm test
  • pnpm lint
  • git diff --check
  • Workflow YAML parsed successfully with Ruby YAML loader
  • Verified no remaining uses: ...@(vN|main|master) references in .github/workflows

Notes

  • Local codeql and actionlint CLIs are not installed here, so final CodeQL/Scorecard closure needs GitHub Actions to rescan this PR.
  • Created the safe-to-review label in the repository for maintainers to opt external fork PRs into bot review.

@github-actions github-actions Bot added area:exporters packages/exporters (PDF/PPTX/ZIP) area:packaging Installers, signing, Linux deb/rpm area:build Turbo/Vite/Biome/tsconfig toolchain labels May 4, 2026
@hqhq1025 hqhq1025 force-pushed the codex/security-code-scanning-fixes branch 2 times, most recently from 4391d0e to 87ffccb Compare May 4, 2026 07:16
@github-actions github-actions Bot added the docs Documentation label May 4, 2026
@hqhq1025 hqhq1025 marked this pull request as ready for review May 4, 2026 07:24
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review mode: initial

Findings

  • [Minor] Dead code: safeFromCodePoint in packages/exporters/src/markdown.ts:319-324 is no longer called after switching to the shared decodeHtmlEntities. Remove this unused function.
    Suggested fix: Delete the safeFromCodePoint function from markdown.ts.
  • [Minor] Orphaned build-snap job in release.yml: the job still runs and uploads an artifact, but provenance and publish no longer depend on it, so the artifact is never consumed. Consider either removing the job or adding a comment explaining why it is intentionally kept as an independent build.
    Suggested fix: Remove the build-snap job and its upload step, or add a comment clarifying that the Snap artifact is published separately (e.g., via a scheduled workflow or manual trigger).

Summary

The PR successfully addresses CodeQL alerts by:

  • Replacing regex-based HTML entity decoding with a shared sequential scanner that avoids ReDoS and correctly preserves literal comparison text (2 < 3).
  • Adding regression tests for both Markdown and PPTX exporters.
  • Pinning all GitHub Actions to commit SHAs (no more floating @vN tags).
  • Setting default workflow permissions to contents: read with minimal job-level elevation.
  • Hardening pull_request_target for fork safety via the safe-to-review label.
  • Clarifying license policy to allow copyleft workflow-only tools (documented in AGENTS.md, CLAUDE.md, and the review prompt).
  • Removing stale workaround code from deploy-website.yml and decoupling Snap build from release gating.

No security regressions, data-loss risks, or new dependencies found. The code changes are well-scoped and forward-compatible.

Testing

  • Two new Vitest tests cover the entity-decoding fix for both Markdown and PPTX exporters.
  • Existing tests for whitespace collapse, tag stripping, and URL entity decoding remain consistent.
  • CI passes with pnpm lint, pnpm typecheck, and pnpm test.

Open-CoDesign Bot

@hqhq1025 hqhq1025 force-pushed the codex/security-code-scanning-fixes branch from 87ffccb to c1d3927 Compare May 4, 2026 07:29
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review mode: follow-up after new commits

Findings

All findings from the previous review are resolved:

  • [Resolved] Dead safeFromCodePoint function in packages/exporters/src/markdown.ts was removed. ✓
  • [Resolved] The build-snap job in release.yml now has an inline comment explaining it is an inspection-only best-effort build, and is no longer orphaned or misleading. ✓

No new issues introduced by this diff. The additional workflow pinning, permission narrowing, base.sha hardening in codex-pr-review.yml, and safe-to-review label gate for fork PRs all align with the PR's stated security hardening goals. The license policy clarification in AGENTS/CLAUDE and the review prompt is consistent and correct.

Summary

This follow-up commit completes the security hardening by pinning all remaining workflow actions to commit SHAs, setting default contents: read across all workflows, and refining the pull_request_target fork safety and prompt-fetching logic. The previous minor findings have been addressed. The PR is ready to merge from a code-review perspective.

Testing

Existing CI checks (pnpm lint, pnpm typecheck, pnpm test) pass. The new entity-decoding tests for Markdown and PPTX were already present in the previous commit; no additional tests needed for the workflow/CI changes.

Open-CoDesign Bot

@hqhq1025 hqhq1025 merged commit b6feab2 into main May 4, 2026
7 checks passed
@hqhq1025 hqhq1025 deleted the codex/security-code-scanning-fixes branch May 4, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:build Turbo/Vite/Biome/tsconfig toolchain area:exporters packages/exporters (PDF/PPTX/ZIP) area:packaging Installers, signing, Linux deb/rpm docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant