Styles arrive through the bundle, and the first components own theirs #87
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ | |
| <style> | ||
| @import url("/fonts/fonts.css"); | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <div id="app"></div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,42 @@ Bun's bundler detects `.module.css` with no configuration and rewrites locally | |
| scoped class names to unique identifiers — checked in Bun's bundler | ||
| documentation, not recalled. | ||
|
|
||
| ### Development serves the built bundle | ||
|
|
||
| The documentation describes `bun build`. Bun's *other* implementation — the | ||
| HTML entry point served by `Bun.serve` — emits the scoped stylesheet correctly | ||
| and never defines the class-name mapping the components import, so every | ||
| component reading one throws and the page renders nothing (oven-sh/bun#18258, | ||
| open since March 2025; fix PR #33405 unmerged as of Bun 1.3.14). Only the dev | ||
| path is affected. | ||
|
|
||
| So `bun run dev` is `scripts/dev.ts`: it bundles into `dist/`, rebuilds on a | ||
| change under `src/`, and starts `server.ts` over the result. `server.ts` no | ||
| longer routes the HTML entry point; it serves `dist/` in development and in | ||
| production alike. It ships ahead of step 1 rather than inside it: it changes | ||
| how the application is served, which is its own reviewable unit. | ||
|
|
||
| That costs hot module replacement and buys a development page that is the | ||
| bundle production ships — so a defect the bundler introduces is under the e2e | ||
| suite rather than only under `build.test.ts`. The asset lookup is | ||
| `dist-routes.ts` rather than inline in `server.ts`, for the reason | ||
| `static-routes.ts` is its own file: its listing guard, which is what keeps a | ||
| request from naming a path outside `dist/`, can then be exercised without | ||
| starting a server. | ||
|
Comment on lines
+97
to
+109
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
for path in scripts/dev.ts server.ts build.test.ts; do
git ls-files --error-unmatch "$path"
done
rg -n -C 8 'watch|Bun\.build|dist|serve|rebuild' scripts/dev.ts server.ts build.test.tsRepository: laidrivm/dota2 Length of output: 10717 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked paths and branch ---'
git branch --show-current
git ls-files --error-unmatch .gitignore
git ls-files 'openspec/changes/*' | sed -n '1,120p'
printf '%s\n' '--- candidate outlines ---'
ast-grep outline scripts/dev.ts --lang typescript
ast-grep outline dist-routes.ts --lang typescript
ast-grep outline build.test.ts --lang typescript
ast-grep outline openspec/changes/file-size-cap/design.md --lang markdown
printf '%s\n' '--- scripts/dev.ts ---'
cat -n scripts/dev.ts
printf '%s\n' '--- dist-routes.ts ---'
cat -n dist-routes.ts
printf '%s\n' '--- build.test.ts ---'
cat -n build.test.ts
printf '%s\n' '--- relevant design section ---'
sed -n '80,125p' openspec/changes/file-size-cap/design.md
printf '%s\n' '--- ignore rules ---'
cat -n .gitignoreRepository: laidrivm/dota2 Length of output: 22065 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
dev = Path("scripts/dev.ts").read_text()
test = Path("build.test.ts").read_text()
route = Path("dist-routes.ts").read_text()
checks = {
"recursive src watcher": bool(re.search(r'watch\([^;\n]*src[^;\n]*\{[^}]*recursive:\s*true', dev)),
"index watcher": bool(re.search(r'watch\([^;\n]*index\.html', dev)),
"initial build before server import": bool(re.search(r'await build\(\).*?await import\(["\']\.\./server\.ts', dev, re.S)),
"source mutation test": bool(re.search(r'(write|append|truncate|rename|rm|unlink)[^(]*\([^)]*(src|index\.html)', test, re.I)),
"dev process exercised by test": bool(re.search(r'(scripts/dev\.ts|run dev|bun\s+dev)', test, re.I)),
"route uses directory listing": bool(re.search(r'listed\(\)\.has\(name\)', route)),
"route canonicalises target": bool(re.search(r'(realpath|resolveReal|realpathSync|canonical)', route, re.I)),
}
for name, value in checks.items():
print(f"{name}: {'yes' if value else 'no'}")
PY
printf '%s\n' '--- all tests mentioning dev, watch, source mutation, or symlink ---'
rg -n -i -C 3 'scripts/dev|run dev|watch|source change|src/|symlink|realpath|traversal|escape' --glob '*.{ts,tsx,js}' .Repository: laidrivm/dota2 Length of output: 39553 🌐 Web query:
💡 Result: Bun provides explicit controls for handling symbolic links in its filesystem operations and runtime, though developers must remain aware of the distinction between filesystem-level traversal and application-level path containment. Bun.Glob Symlink Handling The Bun.Glob class, which provides recursive file scanning, includes options to control symlink traversal [1][2]: - followSymlinks (boolean): When set to true, the scanner will traverse into symbolic link directories [3][2]. It defaults to false [1]. - throwErrorOnBrokenSymlink (boolean): When set to true, the scanner will throw an error if it encounters a broken symbolic link [1][2]. These options are supported in both the synchronous (scanSync) and asynchronous (scan) methods [1]. Bun.file and Path Containment Bun.file returns a reference to a file (a BunFile object) [4]. By default, reading from a BunFile follows symlinks, as this is standard behavior for filesystem operations [5]. Crucially, Bun.file does not natively provide "path containment" or "jail" mechanisms that prevent symlink traversal to locations outside a target directory [5]. If a developer uses Bun.file to access user-supplied paths, they are responsible for ensuring those paths are safe [5]. To implement containment, developers should: - Resolve the canonical path using Bun.realpath before checking if it falls within the expected directory [5]. - Avoid relying solely on string-based path comparisons (like checking if a path starts with a specific prefix), as these are vulnerable to symlink-based path traversal [5]. Security Note While Bun's runtime and internal utilities have implemented robust fixes to prevent symlink-based path traversal in specific contexts—such as archive extraction [6], patch application [7], and module resolution [8][9]—these protections are localized to those features. They do not automatically apply to user-level code using Bun.file or other general-purpose I/O APIs [5]. Developers must proactively validate paths when building features that accept user-controlled filenames or directory paths [5]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
import subprocess
from pathlib import Path
paths = subprocess.check_output(
["git", "ls-files", "-z", "--", "*test.ts", "*test.tsx", "*spec.ts", "*spec.tsx"],
text=False,
).decode().split("\0")
terms = ("scripts/dev.ts", "run dev", "bun dev", "writeFile", "Bun.write",
"appendFile", "renameSync", "rmSync", "unlinkSync", "symlinkSync")
for raw in paths:
if not raw:
continue
text = Path(raw).read_text()
hits = [term for term in terms if term.lower() in text.lower()]
if hits:
print(f"{raw}: {', '.join(hits)}")
PY
printf '%s\n' '--- tracked browser tests ---'
git ls-files -z -- 'e2e/**' | tr '\0' '\n'Repository: laidrivm/dota2 Length of output: 567 🌐 Web query:
💡 Result: Regarding your questions on Bun's handling of symbolic links: 1. Bun.Glob scanSync with followSymlinks: false When using Bun.Glob.scanSync (or scan) with followSymlinks set to false (which is the default), the globber does not traverse into symbolic link directories [1][2]. Regarding whether it returns the symbolic link files themselves, the globber's primary purpose is to match and return file paths based on your pattern [1][3]. If a symbolic link file exists at a location that matches your glob pattern, it will be included in the results, as the restriction only prevents the utility from entering (traversing) directories that are symbolic links [1][4]. 2. Does Bun.file follow symlinks? Yes, Bun.file follows symbolic links [5]. When you pass a path to Bun.file, it treats the path as a reference to the file system location [6]. If that path is a symbolic link, Bun will attempt to access the target file that the link points to [5]. If the target does not exist, an error (such as ENOENT) will be thrown when you attempt to read from the file or check its status, revealing that the operation is attempting to resolve the symlink target [5]. Citations:
Add a rebuild regression test and enforce
🤖 Prompt for AI AgentsSource: MCP tools
Comment on lines
+105
to
+109
Contributor
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
mapfile -t route_files < <(
git ls-files | rg '(^|/)(dist-routes(\.test)?\.ts|server\.ts)$'
)
printf '%s\n' "${route_files[@]}"
rg -n -C 8 'decodeURIComponent|URL|realpath|resolve|relative|symlink|dist' "${route_files[@]}"Repository: laidrivm/dota2 Length of output: 204 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- branch ---'
git branch --show-current
printf '%s\n' '--- tracked route and design files ---'
git ls-files | rg '(^|/)(dist-routes(\.test)?\.ts|server\.ts|design\.md|config\.yaml|CLAUDE\.md)$' || true
printf '%s\n' '--- relevant tracked paths under openspec ---'
git ls-files 'openspec/**' | rg 'file-size-cap|config\.yaml' || trueRepository: laidrivm/dota2 Length of output: 1916 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- dist-routes outline ---'
ast-grep outline dist-routes.ts || true
printf '%s\n' '--- dist-routes source ---'
cat -n dist-routes.ts
printf '%s\n' '--- server references ---'
rg -n -C 12 'dist-routes|serveStatic|static|dist' server.ts
printf '%s\n' '--- route tests and traversal-related tests ---'
git ls-files | rg '(^|/)([^/]*(dist|route|static|server)[^/]*)\.(test|spec)\.(ts|tsx|js|jsx)$' || true
rg -n -C 5 'traversal|symlink|decodeURIComponent|realpath|relative\(|dist-routes' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' . || trueRepository: laidrivm/dota2 Length of output: 9632 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- build.test.ts ---'
cat -n build.test.ts
printf '%s\n' '--- static-routes.ts outline and relevant source ---'
ast-grep outline static-routes.ts || true
cat -n static-routes.ts
printf '%s\n' '--- package and Bun configuration references ---'
git ls-files | rg '(^|/)(package\.json|bun\.lockb?|tsconfig\.json|README\.md)$' | xargs -r -n1 sh -c 'echo "--- $0"; sed -n "1,220p" "$0"'
printf '%s\n' '--- all distFile callers ---'
rg -n -C 5 '\bdistFile\s*\(' --glob '*.ts' --glob '*.tsx' .Repository: laidrivm/dota2 Length of output: 49056 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
from tempfile import TemporaryDirectory
from urllib.parse import urljoin
with TemporaryDirectory() as root:
base = Path(root) / "dist"
outside = Path(root) / "outside.txt"
base.mkdir()
outside.write_text("outside\n")
(base / "asset.js").symlink_to(outside)
listed = {entry.name for entry in base.iterdir()}
requested = "/asset.js"
name = requested[1:]
target = (base / name).resolve()
print(f"listed={sorted(listed)}")
print(f"name_is_listed={name in listed}")
print(f"canonical_target={target}")
print(f"escapes_dist={base.resolve() not in target.parents}")
print(f"opened_content={(base / name).read_text().strip()}")
PYRepository: laidrivm/dota2 Length of output: 279 🌐 Web query:
💡 Result: In Bun, symlink behavior for Glob scanning and file operations is handled as follows: ### Bun.Glob scanSync Symlink Behavior The Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
from tempfile import TemporaryDirectory
with TemporaryDirectory() as root:
base = Path(root) / "dist"
outside = Path(root) / "outside.txt"
base.mkdir()
outside.write_text("outside\n")
(base / "asset.js").symlink_to(outside)
listed = {entry.name for entry in base.iterdir()}
name = "asset.js"
target = (base / name).resolve()
print(f"listed={sorted(listed)}")
print(f"name_is_listed={name in listed}")
print(f"canonical_target={target}")
print(f"escapes_dist={base.resolve() not in target.parents}")
print(f"opened_content={(base / name).read_text().strip()}")
PYRepository: laidrivm/dota2 Length of output: 279 Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') Reachability: External Reject symlink targets outside
🤖 Prompt for AI AgentsSource: MCP tools |
||
|
|
||
| ### A picker rule that reaches into the hero tile becomes a custom property | ||
|
|
||
| Two rules crossed what became a module boundary: the picker rings the tile | ||
| `Enter` would take and fades the tile of a hero already drafted. A scoped class | ||
| name cannot be written from another module's stylesheet, so the tile reads | ||
| `box-shadow: var(--tile-ring, …)` and `opacity: var(--tile-fade, 1)`, and the | ||
| picker sets those two on its own classes. | ||
|
|
||
| Custom properties inherit, so nothing crosses the boundary and specificity | ||
| never enters it. The alternative — a `class` prop on `HeroTile` carrying one of | ||
| the picker's classes — puts two single-class selectors on the same declaration | ||
| and lets emission order decide, which the source does not state. | ||
|
|
||
| Fonts are the exception and do not move at all. `index.html` owns | ||
| `@import url("/fonts/fonts.css")` in an inline `<style>`, so the faces are | ||
| requested from the document rather than from the bundle; `build.test.ts` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { blank } from "./scan.ts"; | ||
|
|
||
| /** `MARK` stands in for whatever a caller is looking for: it survives the scan | ||
| * when it sits in code, and goes when it sits in something the language quotes | ||
| * or comments out. */ | ||
| const kept = (source: string, language: "css" | "ts") => | ||
| blank(source, language).includes("MARK"); | ||
|
|
||
| describe("scanning TypeScript", () => { | ||
| test.each([ | ||
| ["a string", "const a = 'MARK';"], | ||
| ["a line comment", "// MARK"], | ||
| ["a block comment", "/* MARK */"], | ||
| ["template text", "const a = `text MARK`;"], | ||
| ["a regex literal", "const a = /MARK/;"], | ||
| ["a regex after return", "function f() { return /MARK/; }"], | ||
| ["a regex after an arrow", "const f = () => /MARK/;"], | ||
| ["a regex after typeof", "const a = typeof /MARK/;"], | ||
| ])("erases %s", (_, source) => expect(kept(source, "ts")).toBe(false)); | ||
|
|
||
| test.each([ | ||
| ["plain code", "const MARK = 1;"], | ||
| ["a template expression", `const a = \`\${MARK}\`;`], | ||
| ["a nested template expression", `const a = \`\${\`\${MARK}\`}\`;`], | ||
| ["code after an interpolation closes", `const a = \`\${1}\` + MARK;`], | ||
| ["code after a division, which is not a regex", "const a = 1 / 2; MARK;"], | ||
| // A quote that opened no string — one inside a regex literal, say — would | ||
| // otherwise swallow the rest of the file and take the scan silent with it. | ||
| ["code after a quote left open on its line", "const a = 'x\nMARK;"], | ||
| ["code after a comment that names a quote", "// it's fine\nMARK;"], | ||
| ])("keeps %s", (_, source) => expect(kept(source, "ts")).toBe(true)); | ||
|
|
||
| test("preserves offsets, so a match reads back out of the source", () => { | ||
| const source = 'const a = "hidden"; const b = 1;'; | ||
|
|
||
| expect(blank(source, "ts")).toHaveLength(source.length); | ||
| expect(blank(source, "ts").indexOf("const b")).toBe( | ||
| source.indexOf("const b"), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("scanning CSS", () => { | ||
| test.each([ | ||
| ["a comment", "/* MARK */ .a {}"], | ||
| ["a string", '.a { content: "MARK"; }'], | ||
| ])("erases %s", (_, source) => expect(kept(source, "css")).toBe(false)); | ||
|
|
||
| // CSS has neither, and reading one would erase a rule that is really there. | ||
| test.each([ | ||
| ["a line comment", ".a { background: url(//cdn/MARK.png); }"], | ||
| ["a regex literal", ".a { margin: /MARK/; }"], | ||
| ])("keeps what TypeScript would take for %s", (_, source) => | ||
| expect(kept(source, "css")).toBe(true), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not require HTML attribute order.
Line 59 requires
relbeforehref. A valid<link>can place these attributes in either order, so this test can fail without a broken stylesheet link. Match both attributes with order-independent lookaheads. (html.spec.whatwg.org)Based on learnings,
docs/testing.mdsays, “Tests must assert observable behaviour rather than mirror implementation details.” As per path instructions, this rule applies to**/*.{ts,tsx}.Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 58-58: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
<link[^>]+rel="stylesheet"[^>]+href="\\./${sheets[0]}")Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 Prompt for AI Agents
Sources: Path instructions, Learnings