GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(server): apply the realpath containment check to precompressed sidecars by JosunLP · Pull Request #269 · bQuery/bQuery

Conversation

@JosunLP

@JosunLP JosunLP commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • With serveStatic({ precompressed: true }), a .br or .gz sidecar that resolves outside root (for example via a symlink) is now skipped. The next acceptable encoding, or the identity file, is served instead.
  • Hardening only: no API changes, and sidecars inside root behave as before.

Motivation

serveStatic documents that files outside root are never reachable, and since 1.17.0 it re-checks the realpath of the identity file for that reason. The sidecar picked afterwards is a different path, though, and it was stat()ed and streamed without that check.

This is security-relevant, which is why it was reported privately rather than as a public issue. Exploiting it requires a symlink inside the served directory, for example in build output or an upload directory.

Changes included

  • src/server/static.ts: run isInsideRoot() on every sidecar candidate and continue when it fails.
  • tests/server-static.test.ts: fixture linked-sidecar.js with an escaping .br symlink and a real .gz sidecar. The test covers both accept-encoding: br (falls back to identity) and br, gzip (serves the in-root gzip sidecar). It fails on dev.

Validation

  • bunx eslint + bunx prettier
  • bunx tsc --noEmit -p tsconfig.json
  • bun run build (not run)
  • bun test: 3518 pass, 0 fail

Checklist

  • I followed the contributing guidelines.
  • I added or updated tests where behavior changed.
  • I updated documentation when public behavior or developer workflow changed (not needed; the documented guarantee now holds).
  • I confirmed the change does not introduce security-sensitive regressions.

Notes for reviewers

  • A rejected sidecar is skipped silently instead of producing a 403, because the identity file itself is legitimately servable.
  • This costs one extra realpath pair per request, and only when a sidecar exists.
  • Consider publishing a GitHub Security Advisory for 1.17.0 alongside the release.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo


Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@claude
fix(server): apply the realpath containment check to precompressed si…
124a098
…decars

serveStatic() ran its symlink-aware isInsideRoot() check only on the
identity file. With precompressed: true the .br/.gz sidecar chosen
afterwards was stat()ed and streamed without one, so a sidecar that is a
symlink resolving outside root was served, contrary to the documented
guarantee that files outside root are never reachable. Check each sidecar
too and skip it when it resolves outside the root.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo
@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added tests Chenges to the tests server labels Sep 27, 2026
@JosunLP
Merge branch 'dev' into claude/fix-static-sidecar-containment
9cbe2db
@JosunLP
JosunLP marked this pull request as ready for review September 27, 2026 23:55
@JosunLP
JosunLP merged commit 8cb9dd9 into dev Sep 27, 2026
18 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@JosunLP
JosunLP deleted the claude/fix-static-sidecar-containment branch September 27, 2026 23:56
@JosunLP JosunLP mentioned this pull request Sep 28, 2026
release: 1.17.1 #272
Merged
9 tasks done
JosunLP added a commit that referenced this pull request Sep 30, 2026
@JosunLP @claude
release: 1.17.1 (#272)
2d48260
## Summary

- Release PR for **1.17.1**. It brings everything on `dev` since
`1.17.0` into `main`.
- Security-and-correctness patch closing the 1.17.0 audit:
  - three security fixes in `server` and `ssr`;
- ten bug fixes across `core`, `router`, `server`, `reactive`, `forms`,
`store`, `component` and `view`.
- No API removals and no module status transitions. A few behaviour
changes are listed under **Notes for reviewers**.

## Motivation

`dev` is 13 PR merges ahead of `main` (#259–#271). Together they close
issues #246–#253, #256 and #258 (all closed) and the three privately
reported vulnerabilities fixed in #269, #270 and #271.

## Changes included

This PR targets `main`, so the diff shows the whole `1.17.0 → 1.17.1`
delta: **48 files, +2007/−100**. It has three commits on top of `dev`.

**Release commit (`d9d20de`)** contains only metadata and documentation:

- **Version:** `package.json` → `1.17.1`. `AGENT.md`, `llms.txt`,
`.github/copilot-instructions.md`, `.cursorrules` and `.clinerules` are
synced; `check:ai-guidance` passes. `SECURITY.md` already lists
`1.17.x`.
- **Changelog:** `CHANGELOG.md` gets a `[1.17.1] - 2026-09-28` entry
with Security / Fixed / Changed sections.
- **Release notes:** new `docs/release-notes/1.17.1.md`, plus a row in
the release-notes index.
- **Pre-existing doc fixes:**
  - The 1.17.0 release notes page was missing from the sidebar.
- Its date and CHANGELOG anchor said `2026-09-21`; the release date is
`2026-09-25`, so the anchor pointed nowhere.
- **Guides aligned with shipped behaviour:**
- `api-core.md` describes `show()`/`hide()`/`toggle()` and camelCase
`css()` as they work after #262/#263.
  - `router.md` documents decoded params (#260).

**Review-fix commits (`61198bc`, `640830a`)** address the findings from
the review on this PR:

- **Session-bound CSRF (#271):**
- Requests without a stored session use the signed CSRF cookie and never
write to the session. Anonymous traffic can no longer fill the capped
session store or evict real sessions from it.
- A session created during such a request adopts the cookie secret, so
forms rendered earlier still verify.
- A session created or regenerated with `$regenerate()` (a login) always
gets a fresh secret, so a planted cookie cannot reach the authenticated
session.
  - A session removed with `$destroy()` is not written back.
- For a stored session the secret is bound before `next()`, so a token
read later from a streamed body or `ctx.sse()` stays valid.
- **`toggle()` (#263):** `hidden="until-found"` counts as hidden even
with an inline `display`, because it hides through `content-visibility`.
The `api-core.md` list entry now names the inline-`display` exception.

Source changes by PR:

| PR | Change |
| --- | --- |
| #269 | Security: `serveStatic` `realpath` containment for
precompressed sidecars |
| #270 | Security: Node adapter answers handler errors with `500`
instead of crashing; cancels streams and aborts `request.signal` on
disconnect |
| #271 | Security: signed CSRF tokens bound to the session and rotated
on `$regenerate()` (refined in `61198bc`/`640830a`) |
| #259 | `deepEqual`/`isEqual` kind mismatches (#246) |
| #260 | Router: decoded params, encoded static segments (#247) |
| #261 | Server: encoded static route segments (#248) |
| #262 | `css()` camelCase (#249) |
| #263 | `toggle()`/`hide()`/`show()` (#250) |
| #264 | Already-aborted `AbortSignal` in `http` and `listen()` (#251) |
| #265 | Linear-time `email()` (#252) |
| #266 | Trusted Types for component render, `createTemplate()` and the
DOM sanitizer (#253) |
| #267 | `memoryStore()` sweeping, LRU eviction, default cap (#256) |
| #268 | `SECURITY.md` supported line and drift check (#258) |

## Validation

- [x] `bunx eslint .` + `bun run format:check`
- [x] `bun run build`, `check:publish`, `check:size`, `check:package`
- [x] `bun test`: 3568 pass, 0 fail; `test:types` and `lint:types` clean
- [x] `bun run test:coverage` (coverage floor), `bun run check` (incl.
`check:ai-guidance` → 1.17.1) and `bun run docs:build` (no dead links)
- [x] Each regression test added in the review-fix commits fails without
its fix.

## Checklist

- [x] I followed the contributing guidelines.
- [x] I added or updated tests where behavior changed. They were added
in the individual fix PRs and in the review-fix commits.
- [x] I updated documentation when public behavior or developer workflow
changed.
- [x] I confirmed the change does not introduce security-sensitive
regressions.

## Notes for reviewers

**Behaviour changes when upgrading** (also in the CHANGELOG and release
notes):

- **Router params are decoded.** Remove any manual
`decodeURIComponent()`.
- **CSRF with `session()` before `csrf({ secret })`:**
- requests with a stored session get no `bq.csrf` cookie; the secret
lives in the session;
  - visitors without a session keep the signed `bq.csrf` cookie;
- the token rotates on `$regenerate()`, so render a fresh
`csrfToken(ctx)` after login;
  - `bindToSession: false` opts out.
- **`listen({ signal })`** rejects when the signal is aborted before or
during binding.
- **The default session store** evicts beyond 10 000 live sessions.
- **`show()` without an argument** keeps the inline `display` of an
element that is not inline-hidden.
- **`email()`** rejects empty domain labels and addresses over 254
characters.

**Known limitation:** forms rendered to visitors without a session (for
example the login form) rely on the signed cookie, as before 1.17.1. The
server guide recommends a `__Host-` cookie name for that case.

**After merging:** tag `v1.17.1`, publish, and merge `main` back into
`dev`, as after 1.17.0. The review-fix commits exist only on this
branch, so that merge back is required. Consider publishing a GitHub
Security Advisory for ≤ 1.17.0 that covers #269–#271.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo

---
_Generated by [Claude
Code](https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo)_

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved protection against static-file sidecar paths escaping the
configured root and strengthened CSRF protection for session-backed
requests.
* Node server handlers now handle errors and client disconnects more
safely; server startup also responds correctly to cancelled requests.
* Fixed URL-encoded route matching, CSS property handling, visibility
toggling, equality checks, HTTP cancellation, and email validation.
* Expired sessions are cleaned up, and the default in-memory session
store now limits its capacity.

* **Documentation**
* Added version 1.17.1 release notes and updated security guidance and
feature documentation.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 <[email protected]>
This was referenced Sep 30, 2026
fix(server): default the CSRF cookie to the __Host- prefix #274
Merged
release: 1.17.2 #275
Merged
JosunLP added a commit that referenced this pull request Sep 30, 2026
@JosunLP @claude
fix(server): default the CSRF cookie to the __Host- prefix (#274)
64e3944
## Summary

- `csrf()` now names its cookie `__Host-bq.csrf` whenever the cookie
attributes allow it: `Secure`, `Path=/`, no `Domain`. Those are the
defaults.
- Browsers accept a `__Host-` cookie only from the exact host over
HTTPS. A sibling subdomain or a network attacker therefore can no longer
plant or overwrite the CSRF secret, and a planted unprefixed `bq.csrf`
is ignored.

## Motivation

This finishes the last open item of the private advisory on signed CSRF
tokens (cookie tossing). #271 and the 1.17.1 review fixes keep the
secret in the session for visitors who have one. Anonymous forms, such
as the login form, still rely on the cookie. Until now the docs only
*recommended* a `__Host-` name. The advisory's suggested fix was to make
it the default and to document the remaining limitation of unsigned
mode.

The other advisory items are already fixed on `dev`/`main`:
- `serveStatic` sidecar containment: #269.
- Node adapter crash on a throwing handler, and the stream leak on
client disconnect: #270.

## Changes included

- `src/server/csrf.ts`:
- The default cookie name is `__Host-bq.csrf` when `secure: true`,
`path: '/'` and no `domain`. Otherwise it is `bq.csrf`, e.g. with
`secure: false` for local HTTP dev.
- An explicit `cookieName` starting with `__Host-` but with incompatible
attributes throws at startup. Browsers would silently drop such a
cookie, and every unsafe request would fail with `403`.
- `tests/server-stable.test.ts`:
  - New default name.
  - Planted-`bq.csrf` rejection.
  - Fallback name for incompatible attributes.
  - Startup error.
  - Existing assertions updated to the new name.
- `docs/guide/server.md`:
  - The default cookie name and its fallback.
  - The remaining limitation of unsigned (plain double-submit) mode.

## Validation

- [x] `bun run format:check`, `bunx eslint .`, `bun run lint:types`,
`bun run test:types`, `bun run check`
- [x] `bun run build`, `check:publish`, `check:size`, `check:package`
- [x] `bun test`: 3571 pass, 0 fail; `bun run test:coverage`
- [x] `tests/cross-runtime/run.mjs` on Bun and Node: 16/16
- [x] `bun run docs:build`: no dead links
- [x] Without the `csrf.ts` change, the new tests fail: 8 fail,
including the planted-cookie test.

## Checklist

- [x] I followed the contributing guidelines.
- [x] I added or updated tests where behavior changed.
- [x] I updated documentation when public behavior or developer workflow
changed.
- [x] I confirmed the change does not introduce security-sensitive
regressions.

## Notes for reviewers

- **Behaviour change:** with default attributes, the CSRF cookie is
renamed from `bq.csrf` to `__Host-bq.csrf`.
  - Server-side verification is unaffected.
- After deploying, a browser still holding only the old `bq.csrf` cookie
gets a new cookie on its next page load. A form rendered *before* the
deploy fails once with `403`, just like after a secret rotation.
- Client code that reads the cookie directly (unsigned double-submit)
must switch to the new name, or pass `cookieName: 'bq.csrf'`.
- This belongs in the CHANGELOG of the next release (1.17.2 or 1.18.0).
- **Base:** this branch is based on #273 (merge of `main` into `dev`
after 1.17.1), so it builds on the 1.17.1 CSRF code. Until #273 is
merged, this PR's diff also shows the 1.17.1 changes. Please merge #273
first; this PR then shows only its own commit (`10b0649`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo

---
_Generated by [Claude
Code](https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo)_

---------

Co-authored-by: Claude Opus 5 <[email protected]>
JosunLP added a commit that referenced this pull request Oct 1, 2026
@JosunLP @claude
release: 1.17.2 (#275)
acb0eb8
## Summary

- Release PR for **1.17.2**: a security patch that closes cookie tossing
for the CSRF and session cookies.
- `csrf()` and `session()` name their cookies `__Host-bq.csrf` and
`__Host-bq.sid` by default. A sibling subdomain or a network attacker
can therefore no longer plant or overwrite them (#274, `3ecaa91`).
- No API removals and no module status transitions. The two behaviour
changes are listed under **Notes for reviewers**. The session one **logs
users out once**.
- All guides are brought up to date for 1.17.x (`ea9938c`, `a360e96`).

## Motivation

- 1.17.1 keeps the CSRF secret in the session for visitors who have one.
Forms rendered without a session, such as the login form, still relied
on a plantable `bq.csrf` cookie. This was the last open item of the
private advisory on signed CSRF tokens.
- The code review on this PR found that the session cookie `bq.sid` was
open to the same attack. A subdomain could plant a validly signed id for
the attacker's own session. The victim's requests then ran in the
attacker's account (session swapping), and the attacker knew the
matching CSRF token.

## Changes included

The diff against `main` is **27 files, +340/−56**:

- **#274** (`src/server/csrf.ts`, tests, server guide):
- The CSRF cookie defaults to `__Host-bq.csrf` when `secure: true`,
`path: '/'` and no `domain`; otherwise it is `bq.csrf`.
  - The server guide documents the limitation of unsigned mode.
- **Release commit (`8a4fc05`)**:
  - `package.json` → `1.17.2`.
  - AI guidance files synced (`check:ai-guidance` passes).
  - CHANGELOG entry, `docs/release-notes/1.17.2.md`, index and sidebar.
- **Review fixes (`3ecaa91`)**:
- `session()` defaults to `__Host-bq.sid` under the same rule, with
`bq.sid` as the fallback.
- `csrf()` and `session()` throw at startup when a `cookieName` carries
a `__Host-` or `__Secure-` prefix its attributes violate. Prefixes are
matched case-insensitively, as browsers do. `SameSite=None` counts as
`Secure`.
- The shared rule lives in `src/server/cookies.ts`, internal and not
exported.
- Tests cover these cases, and the cross-runtime smoke test expects the
new session cookie name.
- **Docs refresh (`ea9938c`, `a360e96`)**:
  - **Server guide:**
- `ctx.html()` sanitizes without a DOM since 1.17.0, so the old
`happy-dom` shim instructions are removed.
- The opt-out is `{ trusted: true }`; the guide wrongly said `{
sanitize: false }`.
    - `secure: true` is already the session default.
- A pitfall now covers the `__Host-` names, and the version history
covers 1.17.0–1.17.2.
- **Version histories** in the core, reactive, router, forms, store,
component, view, security and ssr guides now cover 1.17.0 and 1.17.1.
- **SSR docs and `llms.txt`:** the CI matrix runs the latest Bun
(`llms.txt` still said Bun 1.3). Bun ≥ 1.4.0 stays the documented
support floor.
- **Trusted Types notes** state that a `trusted-types` directive must
allow the `bquery-sanitizer` policy.

`SECURITY.md` already lists `1.17.x` as supported.

## Validation

- [x] `bun run format:check`, `bunx eslint .`, `bun run lint:types`,
`bun run test:types`
- [x] `bun run check` (incl. `check:ai-guidance` → 1.17.2)
- [x] `bun run build`, `check:publish`, `check:size`, `check:package`
- [x] `bun test`: 3574 pass, 0 fail
- [x] `bun run test:coverage`, run twice
- [x] `bun run docs:build`: no dead links
- [x] `tests/cross-runtime/run.mjs` on Bun and Node: 16/16
- [x] Without the `3ecaa91` source changes, 12 tests fail, including
every new one.
- [x] GitHub CI green on every head, the latest being `a360e96` (27/27).

## Checklist

- [x] I followed the contributing guidelines.
- [x] I added or updated tests where behavior changed.
- [x] I updated documentation when public behavior or developer workflow
changed.
- [x] I confirmed the change does not introduce security-sensitive
regressions.

## Notes for reviewers

- **Behaviour change: CSRF cookie name.** With the default attributes
the cookie is `__Host-bq.csrf` instead of `bq.csrf`.
  - A form rendered before the upgrade fails once with `403`.
- Client code that reads the cookie directly must use the new name or
pass `cookieName: 'bq.csrf'`.
- **Behaviour change: session cookie name, one-time logout.** With the
default attributes the cookie is `__Host-bq.sid` instead of `bq.sid`.
  - Existing sessions are not read any more.
- The old name is deliberately not accepted as a fallback, because that
would reopen session swapping.
- `cookieName: 'bq.sid'` keeps existing sessions for a transition
period.
- **Prefixed names** that the browser would drop now throw at startup.
Such cookies never worked in browsers.
- **Review status:** every review thread is resolved. CodeRabbit
confirmed the fixes in `a360e96`.
- **After merging:**
  - Tag `v1.17.2` and publish.
  - Merge `main` back into `dev` with a merge commit, not a squash.
- Consider publishing the GitHub Security Advisories for ≤ 1.17.1 (CSRF
and session cookie tossing) and for ≤ 1.17.0 (#269–#271).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Security**
* Default CSRF and session cookies now use the `__Host-` prefix when
cookie settings meet its requirements; otherwise, the unprefixed names
remain in use.
* Invalid combinations of prefixed cookie names and settings now produce
startup errors.
* Existing sessions are no longer recognized, so users may need to sign
in again. Older CSRF forms may fail once with a 403, and client code
that accesses the CSRF cookie may need updating.
* **Documentation**
* Updated guides and release notes with cookie compatibility details and
version 1.17.2.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

server tests Chenges to the tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@JosunLP @claude
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.