Skip to content

fix: harden CI permissions and triage CodeQL alerts - #38

Merged
kevincodex1 merged 1 commit into
mainfrom
codex/codeql-cleanup
Sep 13, 2026
Merged

fix: harden CI permissions and triage CodeQL alerts#38
kevincodex1 merged 1 commit into
mainfrom
codex/codeql-cleanup

Conversation

@Vasanthdev2004

@Vasanthdev2004 Vasanthdev2004 commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

A small CodeQL triage and hardening pass against current main, kept separate from the UI/watchlist work in #34. No dependencies, contract behavior, upload behavior or wallet-signature formats change.

  • Give both build-only CI jobs explicit contents: read token permissions, with a regression check.
  • Replace literal invisible control bytes in the post filter with equivalent hexadecimal escapes. Tests cover every UTF-16 code unit, whitespace/Unicode boundaries and the exact signed message.
  • Check sitemap URL origins exactly instead of using a hostname-prefix assertion.
  • Exercise the actual image-preview component through React's HTML serializer, including rejected schemes and quote/markup handling. The component itself is unchanged.

Alert-by-alert scope

CodeQL alert Treatment
#1, #2 One workflow-level read-only policy covers both CI jobs.
#3 Make the intentional control-character range explicit; preserve the existing behavior. This is not a claim of a demonstrated vulnerability.
#5 Tighten a test assertion, not a production URL-validation fix.
#4 Add evidence for maintainer triage. The HTTP(S)-gated value is passed to React's img.src, not an HTML sink. The render tests support a false-positive assessment at this location, but do not constitute a browser/upload security audit. The alert is intentionally left undismissed.

The old posts.ts contains literal NUL/control bytes, so Git may show a binary diff. git diff --text shows the single regex-line replacement.

Validation

  • Clean Linux CI passed: all 340 app tests, lint, typecheck, production build, and contract build/unit checks.
  • CodeQL passed for Actions, JavaScript/TypeScript and Python. Existing main-branch alerts still need post-merge scanning/triage; passing PR checks is not a claim that they have all been closed.
  • 32 focused tests passed, including image decoding/re-encoding tests.
  • Typecheck passed.
  • ESLint passed with the two existing Open Graph image warnings.
  • Full local suite: 338 passed, 2 failed. Both failing tests and their inputs are unchanged from base 532b5eb: launch-machine.test.ts assumes LF line endings, and wallet-picker.test.ts uses URL.pathname as a Windows filesystem path. No unrelated fixes are included.
  • Local Turbopack build was blocked by the separate checkout's shared dependency junction pointing outside its root. The clean Linux CI run above verified the full suite, production build and contract checks.
  • Independent diff review found no blockers. No alert dismissals, scan suppression or scanner configuration changes.

Limit build-only CI tokens to read-only contents permissions.
Clarify intentional control filtering without changing signed post text,
and replace the sitemap test hostname-prefix check with exact origins.

Cover the image-preview React attribute boundary without changing its
runtime behavior or dismissing the existing scanner warning.
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Essentials

Run ID: ca9bd809-3296-4f94-b39c-4ea34d41f87f

📥 Commits

Reviewing files that changed from the base of the PR and between 532b5eb and 6b18a73.

📒 Files selected for processing (6)
  • .github/workflows/ci.yml
  • app/src/components/launchpad/image-upload.test.ts
  • app/src/lib/ci-permissions.test.ts
  • app/src/lib/launchpad/posts.test.ts
  • app/src/lib/launchpad/posts.ts
  • app/src/lib/seo.test.ts

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

The changes add workflow permission coverage and expand tests for image previews, post normalization and signing, and sitemap URL origins.

Changes

CI permission coverage

Layer / File(s) Summary
Workflow permission contract
.github/workflows/ci.yml, app/src/lib/ci-permissions.test.ts
The workflow grants contents: read. Tests verify the workflow-level permission and absence of job-level overrides.

Application validation coverage

Layer / File(s) Summary
Image preview rendering and safety
app/src/components/launchpad/image-upload.test.ts
Tests cover valid HTTP(S) previews, fallback behavior, rejected inputs, and HTML escaping.
Post normalization and signing
app/src/lib/launchpad/posts.test.ts
Tests cover control-character handling, whitespace and Unicode preservation, length limits, and normalized signed messages.
Sitemap origin assertion
app/src/lib/seo.test.ts
The sitemap test compares each entry URL origin with the site origin.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Suggested reviewers: kevincodex1

Merge Risk: ⚪ Minimal · up to 6b18a

The reviewed CI permission, normalization, sitemap, and image-preview test changes introduce no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: hardening CI permissions and addressing CodeQL-related security findings. It is concise and specific.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/codeql-cleanup

Comment @coderabbitai help to get the list of available commands.

@kevincodex1 kevincodex1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@kevincodex1
kevincodex1 merged commit a203587 into main Sep 13, 2026
7 checks passed
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.

2 participants