Skip to content

Fix security issues found in review #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PatAltimore merged 1 commit into from
Jun 22, 2026
+50 −16
Merged

Fix security issues found in review #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions code_generator/checkpointer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
import re
from pathlib import Path

# Slugs are used to build filesystem paths and originate from config that may be
# LLM-generated from an untrusted GitHub repo tree, so restrict them to a safe
# character set to prevent path traversal (e.g. a "../" slug escaping the root).
_SLUG_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$")


def _validate_slug(slug: str) -> str:
if not isinstance(slug, str) or not _SLUG_RE.match(slug):
raise ValueError(f"Unsafe slug: {slug!r}")
return slug


class Checkpointer:
def __init__(self, output_dir: str):
self.root = Path(output_dir)
self.root = Path(output_dir).resolve()

def path_for(self, program_slug: str, file_slug: str) -> Path:
return self.root / program_slug / f"{file_slug}.md"
path = (self.root / _validate_slug(program_slug) / f"{_validate_slug(file_slug)}.md").resolve()
# Defense in depth: ensure the resolved path stays within the root.
if self.root not in path.parents:
raise ValueError(f"Path escapes output root: {path}")
return path

def is_done(self, program_slug: str, file_slug: str) -> bool:
return self.path_for(program_slug, file_slug).exists()
Expand Down
8 changes: 4 additions & 4 deletions public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
</head>
<body>
<div id="app"><div class="loading"></div></div>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/highlight.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/cpp.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/x86asm.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/lisp.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/highlight.min.js" integrity="sha384-F/bZzf7p3Joyp5psL90p/p89AZJsndkSoGwRpXcZhleCWhd8SnRuoYo4d0yirjJp" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/cpp.min.js" integrity="sha384-pF0kkJHJ7iLK/GTjymCseh94/WcMBVV6C9lKk2HdA+dut+ACEizCpQbrzWE4fpfv" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/x86asm.min.js" integrity="sha384-eKsHYF6apOh4wN/zUOnYUx5s4086rCTH2ZG0R09R49HLu6/1Kn/6J7Zn6M8++8On" crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.9.0/build/languages/lisp.min.js" integrity="sha384-+LHHMbAXOUlvquvrQZ9LW4KeR2nwcsh/lpp7xrWu7KuaDSGgAYBIdm8qCw97I1tq" crossorigin="anonymous"></script>
<script src="/js/app.js"></script>
</body>
</html>
33 changes: 24 additions & 9 deletions public/js/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,12 @@ function renderEnhancement(enh) {

const imageHtml = hasImage ? `
<div class="enhancement-image-wrap">
<img src="${escapeAttr(enh.image_url)}" alt="${escapeAttr(enh.title)}" loading="lazy">
<img src="${escapeAttr(safeUrl(enh.image_url))}" alt="${escapeAttr(enh.title)}" loading="lazy">
${enh.image_caption ? `<p class="enhancement-caption">${escapeHtml(enh.image_caption)}${commonsUrl(enh.image_url) ? ` <a class="commons-link" href="${escapeAttr(commonsUrl(enh.image_url))}" target="_blank" rel="noopener">Wikimedia Commons</a>` : ''}</p>` : ''}
</div>` : '';

const wikiHtml = enh.wikipedia_url
? `<a href="${escapeAttr(enh.wikipedia_url)}" target="_blank" rel="noopener">Wikipedia ↗</a>`
? `<a href="${escapeAttr(safeUrl(enh.wikipedia_url))}" target="_blank" rel="noopener">Wikipedia ↗</a>`
: '';

return `
Expand Down Expand Up @@ -396,6 +396,21 @@ function escapeAttr(str) {
return String(str || '').replace(/"/g, '&quot;').replace(/'/g, '&#39;');
}

// Validate a URL before it is placed in an href/src attribute. Content here is
// LLM-generated from untrusted GitHub/Wikipedia sources, so a `javascript:` (or
// `data:`/`vbscript:`) URL could otherwise become stored XSS. Returns the URL
// unchanged when its scheme is allowed (or it has no scheme — i.e. a relative
// or hash link), and '' when the scheme is disallowed.
function safeUrl(url) {
const raw = String(url || '');
// Browsers ignore control chars and whitespace when resolving a scheme, so
// strip them before testing to defeat obfuscation like "java\tscript:".
const probe = raw.replace(/[\u0000-\u0020]+/g, '').toLowerCase();
const scheme = probe.match(/^([a-z][a-z0-9+.-]*):/);
if (scheme && !['http', 'https', 'mailto'].includes(scheme[1])) return '';
return raw.trim();
}

function commonsUrl(url) {
if (!url) return null;
const m = url.match(/\/thumb\/[0-9a-f]\/[0-9a-f]{2}\/(.+?)\/\d+px-/);
Expand Down Expand Up @@ -434,7 +449,7 @@ function renderShelf(catalog) {
<div class="program-meta">
<span>${escapeHtml(p.author)}</span>
<span>·</span>
<span>${p.year}</span>
<span>${escapeHtml(p.year)}</span>
<span>·</span>
<span>${(p.files || []).length} files</span>
</div>
Expand Down Expand Up @@ -577,7 +592,7 @@ function renderProgramPage(program) {

const introImageHtml = program.image_url ? `
<figure class="intro-image">
<img src="${escapeAttr(program.image_url)}" alt="${escapeAttr(program.title)}" loading="lazy">
<img src="${escapeAttr(safeUrl(program.image_url))}" alt="${escapeAttr(program.title)}" loading="lazy">
${program.image_caption ? `<figcaption>${escapeHtml(program.image_caption)}${commonsUrl(program.image_url) ? ` <a class="commons-link" href="${escapeAttr(commonsUrl(program.image_url))}" target="_blank" rel="noopener">Wikimedia Commons</a>` : ''}</figcaption>` : ''}
</figure>` : '';

Expand All @@ -587,13 +602,13 @@ function renderProgramPage(program) {
<div class="program-page">
<div class="program-page-header">
<h1>${escapeHtml(program.title)}</h1>
<div class="byline">${escapeHtml(program.author)} · ${program.year} · ${escapeHtml(program.language)}</div>
<div class="byline">${escapeHtml(program.author)} · ${escapeHtml(program.year)} · ${escapeHtml(program.language)}</div>
${introImageHtml}
${introHtml}
</div>
${highlightsHtml}
<div class="file-tree">${treeHtml}</div>
${program.github_url ? `<a class="github-badge" href="${escapeAttr(program.github_url)}" target="_blank" rel="noopener">View source on GitHub ↗</a>` : ''}
${program.github_url ? `<a class="github-badge" href="${escapeAttr(safeUrl(program.github_url))}" target="_blank" rel="noopener">View source on GitHub ↗</a>` : ''}
</div>`;
}

Expand Down Expand Up @@ -626,7 +641,7 @@ function renderHeader(opts = {}) {
</div>`;
}
if (githubUrl) {
right += `<a class="btn-icon" href="${escapeAttr(githubUrl)}" target="_blank" rel="noopener">GitHub ↗</a>`;
right += `<a class="btn-icon" href="${escapeAttr(safeUrl(githubUrl))}" target="_blank" rel="noopener">GitHub ↗</a>`;
}

return `
Expand Down Expand Up @@ -807,7 +822,7 @@ function renderReader(meta, body, program) {
const nextFile = currentIdx >= 0 && currentIdx < files.length - 1 ? files[currentIdx + 1] : null;

const summaryHtml = (meta.summary || []).map(s =>
`<li>${escapeHtml(s.point)}${s.link ? ` <a href="${escapeAttr(s.link)}" target="_blank" rel="noopener">${escapeHtml(s.link_label || 'Wikipedia')}</a>` : ''}</li>`
`<li>${escapeHtml(s.point)}${s.link ? ` <a href="${escapeAttr(safeUrl(s.link))}" target="_blank" rel="noopener">${escapeHtml(s.link_label || 'Wikipedia')}</a>` : ''}</li>`
).join('');

const codeHtml = renderCodeWithEnhancements(body, meta.enhancements || [], meta.language);
Expand All @@ -825,7 +840,7 @@ function renderReader(meta, body, program) {
<div class="reader-wrap">
<div class="reader-header-meta">
<h1>${escapeHtml(meta.title)}</h1>
<div class="file-byline">${escapeHtml(meta.program)} · ${escapeHtml(meta.language)} · ${meta.year}</div>
<div class="file-byline">${escapeHtml(meta.program)} · ${escapeHtml(meta.language)} · ${escapeHtml(meta.year)}</div>
<p class="file-description">${escapeHtml(meta.description)}</p>
${summaryHtml ? `<ul class="summary-list">${summaryHtml}</ul>` : ''}
${mobileHtml}
Expand Down
5 changes: 4 additions & 1 deletion public/staticwebapp.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
},
"mimeTypes": { ".md": "text/plain" },
"globalHeaders": {
"Referrer-Policy": "no-referrer-when-downgrade"
"Referrer-Policy": "no-referrer-when-downgrade",
"X-Content-Type-Options": "nosniff",
"X-Frame-Options": "DENY",
"Content-Security-Policy": "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://*.wikimedia.org; connect-src 'self'; font-src 'self'; object-src 'none'; base-uri 'self'; frame-ancestors 'none'"
},
"headers": [
{
Expand Down
Loading