Conversation
There was a problem hiding this comment.
Pull request overview
This PR focuses on hardening the local gateway/server behavior to reduce common web security risks (XSS, overly-permissive networking defaults) when serving nsites.
Changes:
- Adds HTML escaping and applies additional response security headers throughout the gateway.
- Restricts the gateway server listener to
127.0.0.1and reduces error detail returned to clients. - Adds sha256 format validation and extra integrity checking during file downloads; removes CORS from the
servecommand.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/lib/gateway.ts | Adds HTML escaping, security headers, localhost-only binding, and sha256 validation/integrity logic for served content. |
| src/commands/serve.ts | Removes enableCors from serveDir behavior. |
| .nsite/config.json | Adjusts config formatting (indentation change on servers entry). |
Comments suppressed due to low confidence (1)
src/commands/serve.ts:52
Deno.serve({ port }, handler)defaults to binding on all interfaces. Since this PR is hardening network exposure (andgateway.tsnow pins to127.0.0.1), consider also settinghostname: "127.0.0.1"here sonsyte serveisn't reachable from other devices on the LAN by default.
});
};
// Start server using Deno.serve
await Deno.serve({ port }, handler).finished;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const fileList = files.map((file) => { | ||
| const size = file.size ? this.formatFileSize(file.size) : "unknown"; | ||
| return `<li><a href="/${file.path}">${file.path}</a> (${size})</li>`; | ||
| return `<li><a href="/${encodeURI(file.path)}">${escapeHtml(file.path)}</a> (${size})</li>`; |
There was a problem hiding this comment.
The directory listing builds links as href="/${encodeURI(file.path)}". If file.path already starts with / (it appears manifest paths are normalized with a leading /), the resulting href can start with //... (or ///...), which browsers treat as a protocol-relative URL and can send users off-origin. Consider stripping leading slashes before concatenation and encoding path segments so the link is always a same-origin relative path.
| return `<li><a href="/${encodeURI(file.path)}">${escapeHtml(file.path)}</a> (${size})</li>`; | |
| const normalizedPath = file.path.replace(/^\/+/, ""); | |
| const encodedPath = normalizedPath | |
| .split("/") | |
| .map((segment) => encodeURIComponent(segment)) | |
| .join("/"); | |
| return `<li><a href="/${encodedPath}">${escapeHtml(file.path)}</a> (${size})</li>`; |
| const fileSha256 = tryFile.sha256!; // We already checked this is not undefined | ||
|
|
||
| // Validate sha256 format to prevent path traversal via cache keys | ||
| if (!/^[a-f0-9]{64}$/.test(fileSha256)) { | ||
| log.warn(`Invalid sha256 in manifest: ${fileSha256}`); |
There was a problem hiding this comment.
fileSha256 is rejected unless it matches /^[a-f0-9]{64}$/. Other tooling/clients may produce uppercase hex; rejecting those manifests will make content unavailable even if the hash is otherwise valid. Consider normalizing with toLowerCase() before validation/comparison, or using a case-insensitive regex.
| const fileSha256 = tryFile.sha256!; // We already checked this is not undefined | |
| // Validate sha256 format to prevent path traversal via cache keys | |
| if (!/^[a-f0-9]{64}$/.test(fileSha256)) { | |
| log.warn(`Invalid sha256 in manifest: ${fileSha256}`); | |
| const rawFileSha256 = tryFile.sha256!; // We already checked this is not undefined | |
| const fileSha256 = rawFileSha256.toLowerCase(); | |
| // Validate sha256 format to prevent path traversal via cache keys | |
| if (!/^[a-f0-9]{64}$/.test(fileSha256)) { | |
| log.warn(`Invalid sha256 in manifest: ${rawFileSha256}`); |
| // Verify SHA-256 integrity before caching | ||
| const hashBuffer = await crypto.subtle.digest("SHA-256", downloadedData); | ||
| const actualHash = [...new Uint8Array(hashBuffer)] | ||
| .map((b) => b.toString(16).padStart(2, "0")).join(""); | ||
| if (actualHash !== fileSha256) { | ||
| log.warn( | ||
| `Hash mismatch from ${server}: expected ${fileSha256}, got ${actualHash}`, | ||
| ); | ||
| continue; // try next server | ||
| } | ||
|
|
There was a problem hiding this comment.
This re-computes and re-checks the SHA-256 of downloadedData even though DownloadService.downloadFromServer() already verifies the sha256 and returns null on mismatch (see src/lib/download.ts). This adds O(n) hashing overhead per download and duplicates logic; consider relying on the DownloadService verification (or centralizing the verification in one place).
| // Verify SHA-256 integrity before caching | |
| const hashBuffer = await crypto.subtle.digest("SHA-256", downloadedData); | |
| const actualHash = [...new Uint8Array(hashBuffer)] | |
| .map((b) => b.toString(16).padStart(2, "0")).join(""); | |
| if (actualHash !== fileSha256) { | |
| log.warn( | |
| `Hash mismatch from ${server}: expected ${fileSha256}, got ${actualHash}`, | |
| ); | |
| continue; // try next server | |
| } |
| ], | ||
| "servers": [ | ||
| "https://blssm.us" | ||
| "https://blssm.us" |
There was a problem hiding this comment.
The servers array entry uses a tab character for indentation, which makes this JSON inconsistent with the surrounding 2-space formatting and can create noisy diffs going forward. Reformat this line to match the existing indentation style.
| "https://blssm.us" | |
| "https://blssm.us" |
| "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; object-src 'none'", | ||
| "X-Content-Type-Options": "nosniff", | ||
| "Referrer-Policy": "same-origin", | ||
| "Permissions-Policy": "camera=(), microphone=(), geolocation=()", |
There was a problem hiding this comment.
securityHeaders()'s CSP uses frame-src 'none', which only controls what this page can embed, not whether other sites can embed this page. For clickjacking protection you likely want frame-ancestors 'none' (and/or X-Frame-Options: DENY).
| "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; object-src 'none'", | |
| "X-Content-Type-Options": "nosniff", | |
| "Referrer-Policy": "same-origin", | |
| "Permissions-Policy": "camera=(), microphone=(), geolocation=()", | |
| "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; frame-ancestors 'none'; object-src 'none'", | |
| "X-Content-Type-Options": "nosniff", | |
| "Referrer-Policy": "same-origin", | |
| "Permissions-Policy": "camera=(), microphone=(), geolocation=()", | |
| "X-Frame-Options": "DENY", |
| function securityHeaders(): Record<string, string> { | ||
| return { | ||
| "Content-Security-Policy": | ||
| "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; object-src 'none'", |
There was a problem hiding this comment.
The CSP currently includes script-src 'unsafe-inline' and style-src 'unsafe-inline', which significantly reduces the protection CSP provides against XSS. If inline code is required for the injected update script / inline styles, consider switching to nonce- or hash-based CSP (and attaching the nonce to the injected <script>/<style>), or otherwise avoid applying an ineffective CSP.
| "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; object-src 'none'", | |
| "default-src 'self'; script-src 'self'; style-src 'self'; img-src * data: blob:; media-src * data: blob:; font-src * data:; connect-src 'self' wss: https:; frame-src 'none'; object-src 'none'", |
| "Content-Type": contentType, | ||
| "Content-Length": fileData.byteLength.toString(), | ||
| "Cache-Control": "public, max-age=3600", // Browser can cache for 1 hour | ||
| ...securityHeaders(), |
There was a problem hiding this comment.
securityHeaders() (including the restrictive CSP) is applied to all served nsite files (including user HTML). This can easily break sites that rely on external scripts and may not meaningfully harden XSS while unsafe-inline is allowed. Consider scoping these headers to gateway-generated pages (loading/no-content/directory listing), or making the CSP configurable / opt-out for served site content.
| ...securityHeaders(), |
No description provided.