Skip to content

fix: prevent wildcard CSS leakage in sb-errordisplay_main #34064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
karimKandil0 wants to merge 2 commits into from
Closed

fix: prevent wildcard CSS leakage in sb-errordisplay_main #34064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ed0d4b
setting up dependancies with flake.nix
karimKandil0 Mar 8, 2026
0379bdc
Fix nested CSS in sb-errordisplay_main to prevent wildcard style leakage
karimKandil0 Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 66 additions & 70 deletions code/core/assets/server/base-preview-head.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,84 +147,80 @@
border: 1px solid #ff0000;
box-shadow: 0 0 64px rgba(0, 0, 0, 0.1);
gap: 24px;
}

& ol {
padding-left: 18px;
margin: 0;
}
.sb-errordisplay_main ol {
padding-left: 18px;
margin: 0;
}

/* Redefine colors to ensure readability regardless of user-provided * selectors. */
* {
background: white;
color: black;
}
/* Prevent wildcard leakage */
.sb-errordisplay_main * {
background: white;
color: black;
}
Comment on lines +157 to +161

@coderabbitai coderabbitai Bot Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep ANSI-rendered stack spans inside the dark code theme.

#error-stack is filled with ansiConverter.toHtml(detail), so the universal reset on Lines 157-160 also hits nested <span> nodes. Right now only the code element is restored on Lines 217-220, which leaves ANSI children with white backgrounds inside the dark <pre>.

Suggested fix 
-  .sb-errordisplay_main .sb-errordisplay_code code {
+  .sb-errordisplay_main .sb-errordisplay_code code,
+  .sb-errordisplay_main .sb-errordisplay_code * {
     background-color: inherit;
     color: inherit;
   }

Also applies to: 217-220

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/assets/server/base-preview-head.html` around lines 157 - 161, The
global reset selector .sb-errordisplay_main * is overwriting ANSI-rendered
<span> children inside `#error-stack`; update the CSS to avoid clobbering those
spans by either narrowing the reset selector or adding an overriding rule that
restores ANSI children inside the error block—e.g., change the broad reset so it
doesn't target `#error-stack` (or use :not(`#error-stack`)) and add/extend the
existing restoration around the code block (references: selector
.sb-errordisplay_main *, the `#error-stack` element and the code restoration rules
near the later block) to include `#error-stack` span { background: transparent;
color: inherit; } so ANSI spans keep the dark code theme.


& h1 {
font-family: Nunito Sans;
font-size: 22px;
font-weight: 400;
line-height: 30px;
font-weight: normal;
margin: 0;

&::before {
content: '';
display: inline-block;
width: 12px;
height: 12px;
background: #ff4400;
border-radius: 50%;
margin-right: 8px;
}
}
.sb-errordisplay_main h1 {
font-family: Nunito Sans;
font-size: 22px;
font-weight: 400;
line-height: 30px;
margin: 0;
}

& p,
& ol {
font-family: Nunito Sans;
font-size: 14px;
font-weight: 400;
line-height: 19px;
margin: 0;
}
.sb-errordisplay_main h1::before {
content: '';
display: inline-block;
width: 12px;
height: 12px;
background: #ff4400;
border-radius: 50%;
margin-right: 8px;
}

& li + li {
margin: 0;
padding: 0;
padding-top: 12px;
}
.sb-errordisplay_main p,
.sb-errordisplay_main ol {
font-family: Nunito Sans;
font-size: 14px;
font-weight: 400;
line-height: 19px;
margin: 0;
}

& a {
color: currentColor;
}
.sb-errordisplay_main li + li {
padding-top: 12px;
}

& .sb-errordisplay_code {
padding: 10px;
flex: 1;
background: #242424;
color: #c6c6c6;
box-sizing: border-box;

font-size: 14px;
font-weight: 400;
line-height: 19px;
border-radius: 4px;

font-family:
'Operator Mono', 'Fira Code Retina', 'Fira Code', 'FiraCode-Retina', 'Andale Mono',
'Lucida Console', Consolas, Monaco, monospace;
margin: 0;
overflow: auto;

& code {
background-color: inherit;
color: inherit;
}
}
.sb-errordisplay_main a {
color: currentColor;
}

& .sb-errordisplay pre {
white-space: pre-wrap;
white-space: revert;
}
.sb-errordisplay_main .sb-errordisplay_code {
padding: 10px;
flex: 1;
background: #242424;
color: #c6c6c6;
box-sizing: border-box;

font-size: 14px;
line-height: 19px;
border-radius: 4px;

font-family:
'Operator Mono', 'Fira Code Retina', 'Fira Code', 'FiraCode-Retina', 'Andale Mono',
'Lucida Console', Consolas, Monaco, monospace;

margin: 0;
overflow: auto;
}

.sb-errordisplay_main .sb-errordisplay_code code {
background-color: inherit;
color: inherit;
}

.sb-errordisplay_main .sb-errordisplay pre {
white-space: revert;
Comment on lines +222 to +223

@coderabbitai coderabbitai Bot Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This pre selector never matches the rendered error DOM.

In code/core/assets/server/base-preview-body.html, .sb-errordisplay_main sits inside .sb-errordisplay, not the other way around, so this rule is dead. That means white-space: revert never applies to the stack block.

Suggested fix 
-  .sb-errordisplay_main .sb-errordisplay pre {
+  .sb-errordisplay_main pre {
     white-space: revert;
   }
📝 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.

Suggested change
.sb-errordisplay_main .sb-errordisplay pre {
white-space: revert;
.sb-errordisplay_main pre {
white-space: revert;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/assets/server/base-preview-head.html` around lines 222 - 223, The
CSS selector is inverted and never matches because .sb-errordisplay_main is
inside .sb-errordisplay; update the rule selector from ".sb-errordisplay_main
.sb-errordisplay pre" to the correct descendant order ".sb-errordisplay
.sb-errordisplay_main pre" (or simply target ".sb-errordisplay pre" if you want
all stack blocks) so that the white-space: revert rule actually applies to the
rendered error stack block.

}

@-webkit-keyframes sb-rotate360 {
Expand Down
18 changes: 18 additions & 0 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
description = "storybook dev environment";

inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";

outputs = { self, nixpkgs }:
let
system = "x86_64-linux";
pkgs = import nixpkgs { inherit system; };
in {
devShells.${system}.default = pkgs.mkShell {
Comment on lines +6 to +11

@coderabbitai coderabbitai Bot Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'flake.nix system key:\n'
nl -ba flake.nix | sed -n '6,15p'

printf '\nRepo-supported architectures from .yarnrc.yml:\n'
rg -n -A6 -B2 'supportedArchitectures|cpu:|os:|- x64|- arm64|- linux|- darwin' .yarnrc.yml

Repository: storybookjs/storybook

Length of output: 146

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'flake.nix (lines 6-15):\n'
sed -n '6,15p' flake.nix | cat -n

printf '\nRepo-supported architectures from .yarnrc.yml:\n'
rg -n 'supportedArchitectures|cpu:|os:|x64|arm64|linux|darwin' .yarnrc.yml | head -20

Repository: storybookjs/storybook

Length of output: 547

Make the dev shell portable across all supported architectures.

The hardcoded system = "x86_64-linux" means nix develop will only work for contributors on Linux x86-64. This conflicts with .yarnrc.yml, which declares support for both darwin and arm64 architectures. Contributors on macOS or ARM64 systems cannot use the dev shell.

Use flake-utils to generate shells for all default systems:

Portable flake pattern 
 {
   description = "storybook dev environment";

   inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
+  inputs.flake-utils.url = "github:numtide/flake-utils";

-  outputs = { self, nixpkgs }:
-  let
-    system = "x86_64-linux";
-    pkgs = import nixpkgs { inherit system; };
-  in {
-    devShells.${system}.default = pkgs.mkShell {
-      packages = with pkgs; [
-        nodejs
-        pnpm
-      ];
-    };
-  };
+  outputs = { nixpkgs, flake-utils, ... }:
+    flake-utils.lib.eachDefaultSystem (system:
+      let
+        pkgs = import nixpkgs { inherit system; };
+      in {
+        devShells.default = pkgs.mkShell {
+          packages = with pkgs; [
+            nodejs
+            pnpm
+          ];
+        };
+      });
 }
📝 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.

Suggested change
outputs = { self, nixpkgs }:
let
system = "x86_64-linux";
pkgs = import nixpkgs { inherit system; };
in {
devShells.${system}.default = pkgs.mkShell {
{
description = "storybook dev environment";
inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
inputs.flake-utils.url = "github:numtide/flake-utils";
outputs = { nixpkgs, flake-utils, ... }:
flake-utils.lib.eachDefaultSystem (system:
let
pkgs = import nixpkgs { inherit system; };
in {
devShells.default = pkgs.mkShell {
packages = with pkgs; [
nodejs
pnpm
];
};
});
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@flake.nix` around lines 6 - 11, The flake currently hardcodes system =
"x86_64-linux" which breaks multi-arch dev shells; replace this with
flake-utils' multi-system pattern by using flake-utils.lib.eachDefaultSystem (or
forAllSystems) to iterate default systems and generate outputs.devShells per
system, import nixpkgs with the system variable inside that loop (e.g., pkgs =
import nixpkgs { inherit system; }), and keep the existing
devShells.${system}.default = pkgs.mkShell block unchanged except for being
created per-system; reference symbols: outputs, system, pkgs, nixpkgs,
devShells, mkShell, and flake-utils.

packages = with pkgs; [
nodejs
pnpm
];
Comment on lines +12 to +15

@coderabbitai coderabbitai Bot Mar 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Declared package manager:\n'
rg -n '"packageManager"\s*:\s*"[^"]+"' package.json

printf '\nYarn-specific repo config:\n'
rg -n 'yarnPath|nodeLinker|npmRegistryServer' .yarnrc.yml

printf '\nflake.nix shell packages:\n'
nl -ba flake.nix | sed -n '11,15p'

Repository: storybookjs/storybook

Length of output: 368

🏁 Script executed:

sed -n '12,15p' flake.nix

Repository: storybookjs/storybook

Length of output: 131

Use the repo's Yarn workflow here, not pnpm.

The repository is explicitly pinned to yarn@4.10.3 in package.json:72, and .yarnrc.yml is already configured for Yarn Berry. Including pnpm in the default shell nudges contributors toward an unsupported install path.

🧹 Minimal fix 
       packages = with pkgs; [
         nodejs
-        pnpm
       ];
📝 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.

Suggested change
packages = with pkgs; [
nodejs
pnpm
];
packages = with pkgs; [
nodejs
];
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@flake.nix` around lines 12 - 15, The default dev shell currently exposes pnpm
(packages list including pnpm) which conflicts with the repo's Yarn Berry
workflow; remove pnpm from the packages array and add the Yarn package instead
(e.g., replace pnpm with yarn or the matching yarn package name provided by your
nixpkgs) so the packages list contains nodejs and yarn, ensuring contributors
use the repo's pinned Yarn version configured via .yarnrc.yml and package.json.

};
};
}