Skip to content

fix: handle base_path in dev server#104

Merged
tylerdave merged 5 commits intomainfrom
dev-server-fix-path
Apr 12, 2026
Merged

fix: handle base_path in dev server#104
tylerdave merged 5 commits intomainfrom
dev-server-fix-path

Conversation

@tylerdave
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 11, 2026

Docs Site preview: https://c3223a91.rockgarden.pages.dev

claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

claude[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

The two security bugs from the previous round are now fixed — the //evil protocol-relative bypass is blocked at validation time, and _strip_base_path correctly uses startswith(base_path + "/") throughout. One minor correctness issue remains.

"<!DOCTYPE html><html><head><meta charset='utf-8'>"
"<title>Rockgarden Dev Server</title></head><body>"
f"<p>This site uses <code>base_path</code>: "
f"<a href='{base_path}/'>{base_path}</a></p>"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Previously flagged and still present (reduced severity): base_path is interpolated without html.escape. The validator now blocks ' and <, which prevents the injection risk, but & is in the allowed character class (r"/[A-Za-z0-9._~:@!$&()*+,;=/-]+") and would produce malformed HTML (e.g. href='/my&blog/' when base_path = "/my&blog"). Using html.escape eliminates this:

from html import escape
# ...
f"<a href='{escape(base_path, quote=True)}/'>{escape(base_path)}</a>"

@tylerdave tylerdave merged commit 656ceba into main Apr 12, 2026
7 checks passed
@tylerdave tylerdave deleted the dev-server-fix-path branch April 12, 2026 18:36
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