Skip to content

fix(public): serve static files in generated Node runtime - #202

Merged
medz merged 2 commits into
mainfrom
fix/node-public-assets
Oct 2, 2026
Merged

medz merged 2 commits into
mainfrom
fix/node-public-assets

Conversation

@medz

@medz medz commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

No description provided.

@netlify

netlify Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for dart-spry canceled.

Name Link
🔨 Latest commit d8c34fc
🔍 Latest deploy log https://app.netlify.com/projects/dart-spry/deploys/6abf12b68326ed00086e3495

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 26 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9c69f505-78fa-4bdb-b221-c377219c8734

📥 Commits

Reviewing files that changed from the base of the PR and between 85beaaa and d8c34fc.

📒 Files selected for processing (1)
  • .github/workflows/public-windows.yml

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e8fa8fd8-83ab-4647-866b-465ced046988

📥 Commits

Reviewing files that changed from the base of the PR and between 84183fb and 85beaaa.

📒 Files selected for processing (5)
  • .github/workflows/test.yml
  • CHANGELOG.md
  • lib/src/public/public.dart
  • lib/src/public/public_runtime_js.dart
  • test/public_runtime_integration_test.dart

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Node.js static-file runtime now imports node:fs, reads metadata through fs.promises.stat, and uses POSIX path semantics for MIME extension detection. Integration tests check generated VM and Node server responses and listener release after shutdown.

Changes

Node static-file runtime

Layer / File(s) Summary
Static-file runtime handling
lib/src/public/public_runtime_js.dart, lib/src/public/public.dart, CHANGELOG.md
The Node.js runtime imports node:fs and reads asset metadata through fs.promises.stat. MIME extension detection uses POSIX path semantics. The changelog describes the runtime updates and associated coverage.
Generated server integration coverage
test/public_runtime_integration_test.dart, .github/workflows/test.yml
Integration tests build and launch generated VM and Node servers, check GET and HEAD responses and missing-file status, and verify listener release after shutdown. The test workflow configures Node.js 24.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 85bea

The static-file runtime change is mergeable after normal checks; no identified issue requires a pre-merge fix.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 85bea

The fix restores expected Node static-file responses. Directory containment remains lexical, so links beneath the public directory can expose files outside it. This is conditional on deployment contents or filesystem-write access; ordinary URL traversal remains blocked.

Retained concerns

  • Medium · security · inferred: Restored Node GET bodies can expose process-readable files outside the configured public directory through an existing or attacker-created filesystem link. Containment checks only the path string before stat and openAsBlob; it does not constrain the physical target. This is conditional link-dependent disclosure, not arbitrary URL traversal. The controls predate this PR, but Node body delivery makes this exposure newly effective.
Security review details

Security Blast Radius

  • inferred — Conditional disclosure is bounded by filesystem links reachable beneath the configured public tree and files readable by the Node process. The evidence does not establish an HTTP mechanism for creating links, cross-tenant write access, or production process privileges.

Security Findings and Attack Paths

  • inferred — A request reaching the public handler can select an otherwise valid relative path whose file or ancestor directory is a link outside the public root. Lexical checks accept that pathname, and restored Node body loading can return the link target. A suitable deployed link or authority to modify the asset tree is required.

Trust Boundaries and Controls

  • observed — The URL-to-filesystem boundary has lexical traversal controls and a regular-file metadata check. Neither establishes physical containment through links. The unchanged VM resolver also uses lexical containment followed by separate metadata and body operations, so this limitation was not introduced across all runtimes.

Resilience and Maintainability Implications

  • observed — The integration test owns its temporary assets, HTTP client, child process, and output subscriptions. Teardown includes bounded process shutdown with a kill fallback and verifies listener release. Ordinary GET, HEAD, and missing-file behavior is covered, but this does not establish link containment or consistency during filesystem replacement.

Hardening Proposals

  • proposed — Define whether the public directory is a physical confinement boundary or a trusted publication tree. If physical confinement is required, use link-safe, root-anchored reads and obtain metadata and body from the same opened object; a canonical-path check alone does not address concurrent replacement. Otherwise, restrict asset-tree writers and validate deployed links explicitly.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately describes the primary change: serving static files in the generated Node runtime.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

A rabbit checks the files at night,
GET brings back the text just right.
HEAD returns an empty view,
Missing paths return 404.
Ports are free when servers cease,
Node and VM can rest in peace.

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

@medz
medz marked this pull request as ready for review October 2, 2026 01:37
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T02:15:40.411031Z d8c34fc Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@medz

medz commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: d8c34fc170

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@medz
medz merged commit 8a49182 into main Oct 2, 2026
7 checks passed
@medz
medz deleted the fix/node-public-assets branch October 2, 2026 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant