Security fixes from scan report #2
Gitzilla review complete
5 issues found.
Some changed files were skipped by repository review settings.
Good - I can confirm that pages.astro (line 114) already has the date fallback that posts.astro is missing. This is an inconsistency that needs to be addressed.
Summary of Findings
Reported 2 issues:
-
[HIGH] The
formatFrontmatterfunction's---escape mechanism is fundamentally broken. The replacement string\\-\\-\\-produces\-\-\-in the file, which is not a valid YAML escape sequence. YAML parsers won't recognize it as an escape, so the original---could still be interpreted as a document separator. This undermines the stated security fix for YAML injection. -
[MEDIUM]
posts.astro'sformatFrontmatteris missing the date fallback thatpages.astrocorrectly has. Ifpost.dateis undefined, the generated YAML will containdate: undefinedwhich is invalid syntax.
Verified as correctly implemented:
- SVG upload rejection in
src/server.ts - Path traversal protection for media deletion
- Slug validation on all API endpoints
- iframe sandbox set to
allow-scriptsonly - DOMPurify
styletag removed from ALLOWED_TAGS andstyleadded to FORBID_ATTR - YAML parsing using
JSON_SCHEMA isProduction()correctly rejects localhost, private IPs, and.localdomains- API token not included in config export
I've reviewed this chunk thoroughly and found two confirmed security vulnerabilities related to YAML injection. Let me summarize:
Summary of Changes in This Chunk:
This PR chunk introduces several security hardening improvements to the StaticPress CMS:
-
Config module (
src/lib/config.ts): Added robustisProduction()validation that properly rejects localhost, private IP ranges (10.x, 172.16-31.x, 192.168.x), and.localhostnames. -
Markdown processing (
src/lib/markdown.ts): Added DOMPurify sanitization with strict tag/attribute allowlists, removed dangerous tags likescript,iframe,style, and addedstyleto forbidden attributes. YAML parsing usesJSON_SCHEMAto prevent arbitrary code execution. -
CSS injection protection (
src/layouts/BaseLayout.astro): Implemented strict whitelist validation for theme CSS values, allowing only safe CSS custom properties with pre-approved color formats, sizes, and keywords. -
Admin security (
src/pages/admin/posts.astro,src/pages/admin/pages.astro): Added duplicate slug validation, improved input sanitization with.trim()checks, and replaced dangerous template literal frontmatter generation with dedicatedescapeYAML()andformatFrontmatter()functions. -
Token handling (
src/pages/admin/deploy.astro): API tokens are now excluded from config file exports for security, and the deploy command shows a shell-safe command pattern for users to run manually instead of executing inline. -
Build page (
src/pages/admin/build.astro): Changed from running builds client-side to displaying thebun run buildcommand for users to execute in their terminal. -
Page/blog routes: Added proper dynamic routing with Astro content collections, SEO meta tags, and proper date handling.
The changes todeploy.astroreorganize the admin panel's configuration handling. The UI now separates site-level settings (URL, Formspree ID) from Cloudflare deployment settings (project name, API token, account ID). The save-config operation intentionally excludes the API token from exports for security, and the deploy command now shows the wrangler CLI invocation with environment variable instructions instead of executing directly. The config file format was restructured to use nestedcloudflareproperties, though this introduces a backward-compatibility gap in the load path that prevents restoring tokens from previously saved configs.
Details
-
[HIGH] YAML frontmatter
---escape produces invalid escape sequence
Location: src/pages/admin/posts.astro:119
TheformatFrontmatterfunction in posts.astro and pages.astro attempts to escape---sequences usingcontent.replace(/^---$/gm, '\\-\\-\\-'). In JavaScript,'\\-\\-\\-'becomes the literal string\-\-\-. However,\-\-\-is NOT a valid YAML escape sequence—YAML only recognizes specific escapes like\n,\t,\", etc. The YAML parser will interpret\-\-\-as literal characters, which does not prevent---from being recognized as a document separator. A user can still inject content that breaks out of frontmatter by starting their post with---.
Suggested fix: Either check if content starts with---and prepend a newline, or use proper YAML block scalar syntax. For example:if (post.content.startsWith('---')) content = '\n' + content; -
[HIGH] YAML injection via unquoted date field in frontmatter generation
Location: src/pages/admin/posts.astro:127
Insrc/pages/admin/posts.astro, theformatFrontmatterfunction generates YAML frontmatter with an unquoteddatefield at line 131:date: ${post.date}. Ifpost.datecontains a newline character, it breaks out of the field value and allows injection of arbitrary YAML keys.Example attack: setting date to
2024-01-01\nauthor: "injected"produces:--- title: "..." date: 2024-01-01 author: "injected" ---
This injects an additional
authorfield into the frontmatter. The same vulnerability exists inpages.astroat line 109 withdate: ${page.date || ...}.Both locations should quote and escape the date value like:
`date: "${escapeYAML(post.date)}"`.
Suggested fix: Quote and escape the date field:date: "${escapeYAML(post.date)}" -
[HIGH] YAML injection via unquoted date field in pages frontmatter generation
Location: src/pages/admin/pages.astro:103
Insrc/pages/admin/pages.astro, theformatFrontmatterfunction at line 109 uses an unquoted date field:date: ${page.date || new Date().toISOString().split('T')[0]}. If the page's date value contains a newline, it injects arbitrary YAML structure. This is the same vulnerability as in posts.astro but affects page exports.The date field should be quoted and escaped:
`date: "${escapeYAML(page.date || new Date().toISOString().split('T')[0])}"`.
Suggested fix: Quote and escape the date field:date: "${escapeYAML(page.date || new Date().toISOString().split('T')[0])}" -
[MEDIUM] Missing fallback for undefined date in formatFrontmatter
Location: src/pages/admin/posts.astro:131
TheformatFrontmatterfunction insertspost.datedirectly without a fallback:date: ${post.date}. While the save function setsdate: new Date().toISOString().split('T')[0], ifpost.dateis undefined (from corrupted localStorage, manual edits, or API responses), this producesdate: undefinedwhich is invalid YAML syntax.
Suggested fix: Use a fallback:date: ${post.date || new Date().toISOString().split('T')[0]} -
[MEDIUM] Config load/save asymmetry breaks token restoration
Location: src/pages/admin/deploy.astro:302
The save-config function explicitly omits the token from the exported JSON for security reasons (line 252-255), but the load-config function only checks forconfig.cloudflare.token(lines 302-306). This creates two problems:- Saved config files can never restore the API token — the user must re-enter it every time
- Old config files using the flat
config.tokenformat are not supported for token restoration
The result is that after exporting a config, users cannot re-import their API token. The load function needs a fallback check for
config.token(the old flat format) to provide backward compatibility for users with existing config files.
Suggested fix: Add a fallback to load token from old flat format:const token = config.cloudflare?.token || config.token;and then restore it to localStorage.