chore(security): CodeQL hardening, supply-chain floors, and Actions least privilege#2831
Merged
davydkov merged 16 commits intoApr 3, 2026
Merged
Conversation
Apply uncle-bob boundary: one policy surface (root overrides) plus small harden helper + tests. pnpm audit: only elliptic (GHSA-848j) remains — no patched release on npm (<0.0.0). Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
… permissions Made-with: Cursor
…l codegen sinks Made-with: Cursor
Made-with: Cursor
fix(security): CodeQL icons literals + pnpm overrides for advisories
- Use full zx@8.8.5 (catalog + override) instead of lite variant for GHSA-w87r-vg9q-crqm - to-html.spec: stable HTML tag strip for incomplete sanitization query - language-server _merge: skip prototype-pollution keys - vite-plugin icons: codeql[js/bad-code-sanitization] justification for hardened import specifiers Made-with: Cursor
fix(security): CodeQL findings and zx 8.8.5 resolution
…-utility - Skip __proto__, constructor, prototype with literal comparisons - Only deep-merge when key is an own property of target (CodeQL CWE-915 guidance) Made-with: Cursor
…lution fix(language-server): harden _merge for CodeQL prototype-pollution-utility
🦋 Changeset detectedLatest commit: 2a92210 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Upstream workflow should not reference external forks; workflow_dispatch remains for manual runs. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8d9430865
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- hardenJsonStringLiteralForEmbeddedScript: replaceAll + \\u005C replacements - _merge: Object.hasOwn for own-property check - ci-pr/push: actions: write on caller so checks.yaml artifact jobs keep scope Made-with: Cursor
2e8fd8e to
8ff1556
Compare
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughAdds a JSON-string literal hardening utility and applies it across vite-plugin virtual modules, tightens an internal module merge helper to avoid prototype pollution, enforces least-privilege GitHub Actions permissions, and adds pnpm transitive dependency overrides plus related metadata and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin/src/virtuals/single-project.ts (1)
5-16:⚠️ Potential issue | 🟠 MajorUse SAFE_PROJECT_ID_REGEX to validate id before splicing into import specifiers.
Lines 8 and 15 splice
iddirectly into staticfromspecifiers without validation. Unlikeicons.tswhich validates project IDs withSAFE_PROJECT_ID_REGEX,single-project.tshas no guard. Ifidcontains a quote or special character, the generated module breaks.The proposed fix above is syntactically invalid—static import specifiers cannot accept expressions (e.g.,
from ${variable}is not valid JavaScript). Valid solutions:
- Apply
SAFE_PROJECT_ID_REGEXvalidation toidbefore callingcode(), matching the pattern used inicons.ts.- Use dynamic imports via
import()for the module specifiers (already used in_shared.tsfor combined projects).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin/src/virtuals/single-project.ts` around lines 5 - 16, The template builder code(id: string) inserts id directly into static import specifiers (export ... from 'likec4:icons/${id}' and 'likec4:model/${id}') which can break if id contains unsafe characters; update code() to validate id against SAFE_PROJECT_ID_REGEX (same check used in icons.ts) and throw or reject when it doesn't match before creating projectIdLiteral with hardenJsonStringLiteralForEmbeddedScript, ensuring only safe IDs are spliced into the static specifiers; do not attempt to convert the specifiers to expressions (static imports must remain string literals) — alternatively, if dynamic loading is desired, switch these to dynamic import() calls elsewhere rather than using template-spliced static imports.
🧹 Nitpick comments (2)
.github/workflows/codeql.yml (1)
23-24: Consider whether hardcoding a specific contributor fork is the right long-term approach.The condition now allows
sraphaz/likec4in addition tolikec4/likec4. While this is useful for the current contributor to validate CodeQL fixes, hardcoded fork names can become stale if the contributor leaves or renames their fork.An alternative pattern is to allow any fork when triggered via
workflow_dispatchwhile restricting scheduled runs to the upstream repo:if: ${{ github.repository == 'likec4/likec4' || github.event_name == 'workflow_dispatch' }}This is optional—keeping the explicit fork is fine if coordinated with the contributor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/codeql.yml around lines 23 - 24, The workflow hardcodes a single contributor fork in the if conditional (the line checking github.repository), which can become stale; update the conditional in .github/workflows/codeql.yml (the if that currently checks github.repository) to stop listing a specific fork and instead allow runs when the repo is the upstream (github.repository == likec4/likec4) OR when the workflow was manually triggered (github.event_name == workflow_dispatch) so contributors can use workflow_dispatch without hardcoding forks.packages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.spec.ts (1)
52-57: Consider adding a test for objects/arrays containing special characters.The current test verifies that objects/arrays are accepted, but doesn't verify that special characters within them are escaped correctly.
🧪 Optional test case to strengthen coverage
it('accepts JSON.stringify of object and array roots', () => { const obj = JSON.stringify({ a: 1 }) expect(hardenJsonStringLiteralForEmbeddedScript(obj)).toBe(obj) const arr = JSON.stringify([1, 2]) expect(hardenJsonStringLiteralForEmbeddedScript(arr)).toBe(arr) }) + + it('escapes special characters inside object values', () => { + const obj = JSON.stringify({ url: 'http://example.com/<path>' }) + const out = hardenJsonStringLiteralForEmbeddedScript(obj) + expect(out).toContain('\\u002F') + expect(out).toContain('\\u003C') + expect(out).toContain('\\u003E') + expect(JSON.parse(out)).toEqual({ url: 'http://example.com/<path>' }) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.spec.ts` around lines 52 - 57, Add unit tests for hardenJsonStringLiteralForEmbeddedScript that serialize objects and arrays containing special characters (e.g., HTML-sensitive sequences like "</" and "<!--", line/paragraph separators \u2028 and \u2029, quotes and backslashes) and assert the returned string is the hardened/escaped form expected; include both an object and an array case, and check the specific escapes (e.g., "</" -> "<\/", \u2028/\u2029 properly escaped) to ensure embedded-script safety for these characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.ts`:
- Around line 24-40: The hardening pass only updated one place; other virtual
modules (puml, dot, previewsSources) still interpolate raw JSON.stringify(...)
output into generated JS, leaving the same sink; locate where those modules
embed JSON.stringify(...) output and wrap the stringified value with
hardenJsonStringLiteralForEmbeddedScript(...) (the function exported as
hardenJsonStringLiteralForEmbeddedScript) and ensure you import that function
into each module, replacing direct .replace hacks or raw interpolation with the
hardened wrapper so all embedded JSON literals are escaped consistently.
- Around line 9-22: The function looksLikeJsonStringifyOutput currently uses
prefix heuristics that let malformed JSON through; replace that logic with a
real JSON parse check: ensure the input is a non-empty string, then attempt
JSON.parse(s) inside a try/catch and return true only if parsing succeeds
(return false on any exception); update the function body for
looksLikeJsonStringifyOutput to use this JSON.parse-based validation so only
valid JSON values (objects, arrays, strings, numbers, true/false/null) pass the
guard.
---
Outside diff comments:
In `@packages/vite-plugin/src/virtuals/single-project.ts`:
- Around line 5-16: The template builder code(id: string) inserts id directly
into static import specifiers (export ... from 'likec4:icons/${id}' and
'likec4:model/${id}') which can break if id contains unsafe characters; update
code() to validate id against SAFE_PROJECT_ID_REGEX (same check used in
icons.ts) and throw or reject when it doesn't match before creating
projectIdLiteral with hardenJsonStringLiteralForEmbeddedScript, ensuring only
safe IDs are spliced into the static specifiers; do not attempt to convert the
specifiers to expressions (static imports must remain string literals) —
alternatively, if dynamic loading is desired, switch these to dynamic import()
calls elsewhere rather than using template-spliced static imports.
---
Nitpick comments:
In @.github/workflows/codeql.yml:
- Around line 23-24: The workflow hardcodes a single contributor fork in the if
conditional (the line checking github.repository), which can become stale;
update the conditional in .github/workflows/codeql.yml (the if that currently
checks github.repository) to stop listing a specific fork and instead allow runs
when the repo is the upstream (github.repository == likec4/likec4) OR when the
workflow was manually triggered (github.event_name == workflow_dispatch) so
contributors can use workflow_dispatch without hardcoding forks.
In
`@packages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.spec.ts`:
- Around line 52-57: Add unit tests for hardenJsonStringLiteralForEmbeddedScript
that serialize objects and arrays containing special characters (e.g.,
HTML-sensitive sequences like "</" and "<!--", line/paragraph separators \u2028
and \u2029, quotes and backslashes) and assert the returned string is the
hardened/escaped form expected; include both an object and an array case, and
check the specific escapes (e.g., "</" -> "<\/", \u2028/\u2029 properly escaped)
to ensure embedded-script safety for these characters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab039477-a284-48d8-9f10-a8f63fafcad5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.changeset/security-icons-codeql-lockfile.md.github/workflows/checks.yaml.github/workflows/ci-pr.yaml.github/workflows/codeql.yml.github/workflows/issue-comment.yaml.github/workflows/pkg-pr-new.yaml.github/workflows/push.yaml.github/workflows/trigger-deploy-template.yaml.gitignorepackage.jsonpackages/core/src/utils/markdown/to-html.spec.tspackages/language-server/src/module.tspackages/vite-plugin/src/virtuals/_shared.tspackages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.spec.tspackages/vite-plugin/src/virtuals/hardenJsonStringLiteralForEmbeddedScript.tspackages/vite-plugin/src/virtuals/icons.tspackages/vite-plugin/src/virtuals/mmd.tspackages/vite-plugin/src/virtuals/single-project.tspackages/vscode/package.json
- CodeQL: scheduled runs only on upstream; workflow_dispatch allowed on any fork - Validate JSON.stringify output with JSON.parse in harden helper - Harden puml, dot, and previewsSources embedded literals - Add tests for malformed pseudo-JSON inputs Made-with: Cursor
Collaborator
Author
|
@chatgpt-codex-connector Addressed: Both workflows that invoke the reusable
This matches the GitHub rule that a called workflow cannot raise permissions above the caller. |
|
To use Codex here, create an environment for this repo. |
…ptions Made-with: Cursor
Made-with: Cursor
davydkov
approved these changes
Apr 3, 2026
Member
|
Nice one! Thank you! |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Reduce GitHub security noise and real risk: clear or prevent Dependabot alerts on transitive npm packages, fix Code scanning (CodeQL) findings in our code and generated virtual modules, and tighten GitHub Actions permissions and checkout behavior.
1. Supply chain (
package.json,pnpm-lock.yaml, changeset)pnpm.overrides: raise version floors for known vulnerable transitives (e.g.lodash/lodash-es,path-to-regexp,picomatch,brace-expansionchains,bn.jswhere constrained,yaml,smol-toml,ajv,pbkdf2,sha.js,min-document,diff,express/url→qs,rollup, and related). Goal: align the lockfile with patched releases where npm publishes them.pnpm-lock.yaml: regenerated to match overrides..changeset/security-icons-codeql-lockfile.md): patch bump forlikec4documenting security-related hardening of the published CLI/package surface tied to these changes.2. Vite plugin — embedded literals & CodeQL (
js/bad-code-sanitization)hardenJsonStringLiteralForEmbeddedScript: new helper (+ unit tests) that hardens the output ofJSON.stringify(...)when pasted into generated JS (e.g. dynamicimport(...)), escaping characters that can break out of string/script context (<,>,/, line/paragraph separators). Includes a contract guard so callers only pass realJSON.stringifyoutput.icons,_shared/generateCombinedProjects,single-project,mmdso CodeQL sees consistent sanitization of embedded specifiers/literals.3. CodeQL test & Langium merge (JavaScript/TypeScript queries)
packages/core/.../to-html.spec.ts: replace a single-pass.replace(/<[^>]*>/g, '')with astripHtmlTagshelper that repeats until stable, addressingjs/incomplete-multi-character-sanitizationin tests.packages/language-server/src/module.ts—_merge: harden the Langium-copied DI merge forjs/prototype-pollution-utility:__proto__,constructor,prototypevia literal comparisons (CodeQL CWE-915 examples);target(Object.prototype.hasOwnProperty.call(target, key)), avoiding recursion into prototype-chain objects.iconsvirtual module:// codeql[js/bad-code-sanitization]comment documenting why generatedimport(...)arguments are safe (allowlisted project ids +JSON.stringify+ harden).4. Dependabot —
zx(GHSA-w87r-vg9q-crqm)@likec4/vscode: depend onzxvia workspacecatalog:(full8.8.5) instead oflite(8.8.5-lite), matching the patched release line in the advisory."zx": "8.8.5"so resolution stays on the fixed full package.5. GitHub Actions (least privilege & TOCTOU)
codeql.yml: keep scheduled analysis; addworkflow_dispatchfor manual runs;ifso the job runs onlikec4/likec4or contributor forksraphaz/likec4(same signal when developing on a fork).issue-comment.yaml: checkout PR head using immutablehead.shafrom the API andpersist-credentials: falsewhere appropriate to mitigate untrusted checkout / TOCTOU concerns.checks.yaml: defaultcontents: read; restrictactions: writeto jobs that upload/download artifacts so least privilege without breaking artifact steps.ci-pr.yaml,push.yaml,pkg-pr-new.yaml,trigger-deploy-template.yaml: explicitpermissionswhere needed for callers or standalone workflows.6. Chore
.gitignore: ignore local.gh-pr-*.md/.gh-pr-*.txtdraft files (local-only; not shipped).Dependabot note —
elliptic(CVE-2025-14505)ellipticversion on npm at this time; dependency is transitive (e.g. viabrowserify-sign). This PR does not claim to remove that alert. Maintainers may dismiss the advisory with a documented risk acceptance until upstream ships a fix or the bundler/polyfill chain drops the package.Verification (local)
pnpm installpnpm turbo run typecheck(relevant packages)vitestforhardenJsonStringLiteralForEmbeddedScript.spec.tsandto-html.spec.tsPost-merge (maintainers)
ellipticalert if policy accepts transitive/no-fix state.