Open
Conversation
Address all findings from the 2026-04-11 security evaluation: HIGH: - SEC-001: Replace unbounded sync.Map PathCache with bounded LRU (hashicorp/golang-lru) to prevent memory exhaustion DoS MEDIUM: - SEC-003: Make panic stack traces configurable via STATIC_DEBUG env var - SEC-004: Generate random multipart boundary per response (crypto/rand) - SEC-005: Add MaxCompressSize (10MB) limit for on-the-fly gzip - SEC-006: Apply path.Clean in CacheKeyForPath to prevent cache poisoning LOW: - SEC-007: Suppress server name disclosure (empty Name field) - SEC-008: Sanitize control characters in access log URIs - SEC-009: Remove deprecated PreferServerCipherSuites TLS option - SEC-010: Handle template execution errors in directory listing - SEC-011: Add MaxServeFileSize (1GB) hard limit for large file serving - SEC-012: Add clarifying comment on CORS wildcard Vary behavior - SEC-013: Document ETag 64-bit truncation rationale - SEC-014: Set explicit MaxRequestBodySize (1024 bytes) - SEC-015: Add MaxConnsPerIP config support for rate limiting - SEC-016: Validate symlink targets during cache preload Also updates dependencies: - brotli v1.2.0 → v1.2.1 - klauspost/compress v1.18.4 → v1.18.5 - fasthttp v1.69.0 → v1.70.0
- Landing page (docs/index.html): add 3 new config fields to tables, update security tabs (DoS, TLS, runtime), architecture pipeline descriptions, and feature cards - README.md: update architecture diagram, config tables (+3 fields), env vars (+3 vars), DoS mitigations, and path-safety cache design - USER_GUIDE.md: update config example, env vars table, preload section (symlink validation, bounded LRU), add 413 troubleshooting entry - config.toml.example: add max_compress_size to [compression] section - CHANGELOG.md: add v1.6.2 entry covering all 16 security fixes, dependency bumps, and documentation updates
Update SECURITY_EVAL_2026-04-11.md with resolution status for each finding, upgrade overall grade from B+ to A, expand remediation plan table with Status column covering all 16 items (previously grouped SEC-012–016 as backlog).
There was a problem hiding this comment.
Pull request overview
This PR applies the remediations from the 2026-04-11 security audit to harden the static-web Go server against several DoS, information disclosure, and cache consistency issues, and updates documentation/config examples accordingly.
Changes:
- Replace the unbounded path-safety cache with a bounded LRU and pre-warm it on startup.
- Add resource-limit controls (per-IP connection cap, max servable file size, max on-the-fly compression size) and tighten server defaults (suppress
Serverheader, limit request body size). - Improve security hygiene (random multipart range boundaries, URI log sanitization, configurable panic stack trace verbosity, symlink validation during preload) and update docs/audit report.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| USER_GUIDE.md | Documents new config/env knobs and adds 413 troubleshooting guidance. |
| SECURITY_EVAL_2026-04-11.md | Adds the full security audit report and remediation status. |
| README.md | Updates architecture/feature docs to reflect new security hardening behavior. |
| internal/server/server.go | Suppresses Server header, sets MaxRequestBodySize, wires MaxConnsPerIP, updates TLS commentary. |
| internal/security/security.go | Replaces sync.Map path cache with bounded golang-lru PathCache; expands CORS wildcard note. |
| internal/headers/headers.go | Canonicalizes cache keys via path.Clean while preserving directory semantics. |
| internal/handler/middleware.go | Adds URI control-char sanitization for logs and gates panic stack traces behind STATIC_DEBUG=1. |
| internal/handler/file.go | Enforces max servable file size, documents ETag truncation rationale, randomizes multipart boundaries. |
| internal/handler/dirlist.go | Handles directory listing template execution errors instead of discarding them. |
| internal/config/config.go | Adds config fields/defaults/env overrides for MaxConnsPerIP, MaxServeFileSize, MaxCompressSize. |
| internal/compress/compress.go | Skips on-the-fly gzip when body exceeds MaxCompressSize. |
| internal/cache/preload.go | Validates symlink targets during preload to prevent preloading outside-root targets. |
| cmd/static-web/main.go | Updates PathCache construction for new bounded cache API. |
| config.toml.example | Documents new config fields and defaults in the example config. |
| docs/index.html | Updates documentation site content to reflect the new hardening features and defaults. |
| go.mod / go.sum | Bumps dependency versions (fasthttp, klauspost/compress, brotli) and retains golang-lru usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
22
to
41
| func CacheKeyForPath(urlPath, indexFile string) string { | ||
| if indexFile == "" { | ||
| indexFile = "index.html" | ||
| } | ||
| if urlPath == "" || urlPath == "/" { | ||
| // Remember whether the original path denotes a directory (trailing slash). | ||
| isDir := urlPath == "" || urlPath == "/" || strings.HasSuffix(urlPath, "/") | ||
|
|
||
| // SEC-006: Normalise the URL path to prevent cache-key collisions caused | ||
| // by non-canonical input (e.g. "/a/../b" should map to the same key as "/b"). | ||
| urlPath = path.Clean("/" + urlPath) | ||
|
|
||
| if urlPath == "/" { | ||
| if indexFile == "index.html" { | ||
| return "/index.html" // static string — zero alloc | ||
| } | ||
| return "/" + indexFile | ||
| } | ||
| if strings.HasSuffix(urlPath, "/") { | ||
| return urlPath + indexFile | ||
| if isDir { | ||
| return urlPath + "/" + indexFile | ||
| } |
There was a problem hiding this comment.
CacheKeyForPath now canonicalizes with path.Clean, but headers_test currently only covers basic root/directory/file cases. Add test cases for non-canonical inputs (e.g. "/a/../b", repeated slashes, and "/dir/./") to lock in the intended normalization and trailing-slash semantics.
Comment on lines
+47
to
69
| // DefaultPathCacheSize is the default maximum number of entries in the | ||
| // PathCache. 10 000 entries ≈ 1–2 MB of memory for typical URL paths. | ||
| const DefaultPathCacheSize = 10_000 | ||
|
|
||
| // PathCache caches the results of PathSafe so that repeated requests for the | ||
| // same URL path skip the filesystem syscalls (filepath.EvalSymlinks). | ||
| // It is safe for concurrent use. | ||
| // | ||
| // SEC-001: Uses a bounded LRU cache (hashicorp/golang-lru) instead of an | ||
| // unbounded sync.Map to prevent memory exhaustion via cache-flooding attacks. | ||
| type PathCache struct { | ||
| m sync.Map // urlPath (string) → safePath (string) | ||
| cache *lru.Cache[string, string] | ||
| } | ||
|
|
||
| // NewPathCache creates a new empty PathCache. | ||
| func NewPathCache() *PathCache { | ||
| return &PathCache{} | ||
| // NewPathCache creates a new PathCache with the given maximum number of entries. | ||
| // When the cache is full, the least-recently-used entry is evicted. | ||
| func NewPathCache(maxEntries int) *PathCache { | ||
| if maxEntries <= 0 { | ||
| maxEntries = DefaultPathCacheSize | ||
| } | ||
| c, _ := lru.New[string, string](maxEntries) | ||
| return &PathCache{cache: c} | ||
| } |
There was a problem hiding this comment.
PathCache was changed from an unbounded sync.Map to a bounded LRU, but internal/security/security_test.go has no coverage for the cache behavior. Add a test that inserts more than maxEntries and asserts Len() never exceeds the configured capacity (and ideally that lookups still work for recently-used keys).
Comment on lines
68
to
76
| s.http = &fasthttp.Server{ | ||
| Handler: httpHandler, | ||
| Name: "static-web", | ||
| Name: "", // SEC-007: suppress server identity disclosure | ||
| ReadTimeout: cfg.ReadTimeout, | ||
| WriteTimeout: cfg.WriteTimeout, | ||
| IdleTimeout: cfg.IdleTimeout, | ||
| MaxRequestBodySize: 0, // no request bodies for a static server | ||
| MaxRequestBodySize: 1024, // SEC-014: explicit small limit (static server has no request bodies) | ||
| MaxConnsPerIP: cfg.MaxConnsPerIP, // SEC-015: per-IP connection rate limiting | ||
| DisableKeepalive: false, |
There was a problem hiding this comment.
Security-hardening defaults (suppressed Server header, MaxRequestBodySize=1024, and MaxConnsPerIP wiring) are important behavior but aren't asserted in internal/server/server_test.go. Consider adding a unit test that constructs a Server and verifies these fasthttp.Server fields are set as expected (for both HTTP-only and TLS-enabled setups).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.