Conversation
avoidwork
commented
Feb 25, 2026
avoidwork
commented
- Random fixes
- Simplifying README.md
…hance path traversal protection Changes made: - Removed automatic X-Powered-By header to prevent server information leakage - Fixed etags validation to use !== null check - Enhanced IPv6 validation with stricter regex pattern - Improved path traversal protection using realpath for symlink handling - Fixed parse function to properly default connection IP - Created PLAN.md documenting audit progress and remaining work Current status: 358/367 tests passing (9 failing) Coverage: 99.79% (99.18% branches) Security improvements completed
…andling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrote README.md to accurately reflect the actual source code in the src directory. Removed extensive benchmark data, security assessments, and testing coverage details that weren't reflected in the actual implementation. New README focuses on: - Core HTTP framework functionality - Routing with parameters and RegExp - Middleware support (Express-compatible) - Static file serving - CORS and security features - CLI tool - Complete API reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Augment PR SummarySummary: This PR bundles several small security/correctness fixes across Woodland and streamlines project documentation. Changes:
Technical Notes: The included test run shows 367 tests passing with 100% coverage. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| const port = host.includes(":") && !host.startsWith("[") ? host.split(":")[1] : "8000"; | ||
| const connectionIP = arg.connection?.remoteAddress || arg.socket?.remoteAddress || "localhost"; | ||
|
|
||
| return new URL(`${arg.url}`, `http://${connectionIP}:${port}`); |
There was a problem hiding this comment.
parse() builds the base URL from connectionIP, which will throw when remoteAddress is IPv6 (e.g. ::1 / ::ffff:127.0.0.1) because IPv6 needs bracket notation in URLs. Since parse() is called during decorate() without a try/catch, this can crash request handling on IPv6 connections.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| const host = arg.headers?.host || "localhost:8000"; | ||
| const port = host.includes(":") && !host.startsWith("[") ? host.split(":")[1] : "8000"; |
There was a problem hiding this comment.
Port extraction from host skips bracketed IPv6 hosts like [::1]:3000, so port falls back to 8000 even when the Host header specifies a different port. That makes req.parsed.port inconsistent with the incoming request for IPv6 hosts.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const ipv4MappedMatch = IPV4_MAPPED_PATTERN.exec(ip); | ||
| if (ipv4MappedMatch) { | ||
| return isValidIP(ipv4MappedMatch[1]); | ||
| return isValidIP(ipv4MappedMatch[0].replace(/^::ffff:/, "")); |
There was a problem hiding this comment.
In the IPv4-mapped IPv6 branch, replace(/^::ffff:/, "") is case-sensitive while IPV4_MAPPED_PATTERN is case-insensitive, so inputs like ::FFFF:127.0.0.1 will recurse with the same value and can overflow the stack. This looks attacker-controllable via headers such as X-Forwarded-For.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const IPV4_MAPPED_PATTERN = /^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/i; | ||
| const HEX_GROUP_PATTERN = /^[0-9a-fA-F]{1,4}$/; | ||
| const IPV4_PATTERN = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/; | ||
| const IPV6_CHAR_PATTERN = /^(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|::(?:[0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}|[0-9a-fA-F]{1,4}::(?:[0-9a-fA-F]{1,4}:){0,5}[0-9a-fA-F]{1,4}|(?:[0-9a-fA-F]{1,4}:){1,7}:|::1|::$/; |
There was a problem hiding this comment.
Because IPV6_CHAR_PATTERN isn’t grouped/anchored as ^(?:...|...|...)$, test() can succeed on substrings and allow non-hex characters elsewhere in the string to slip past this “quick validation”. Combined with the non-compressed branch only checking group.length > 4, some invalid full-notation IPv6 strings may be accepted.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| try { | ||
| // Use realpath to resolve symlinks and normalize the path | ||
| fpPath = await realpath(fp); |
There was a problem hiding this comment.
Only fp is resolved via realpath(), but folder isn’t, so if the configured folder is itself a symlink the subsequent containment comparison can reject legitimate files (or compare inconsistent roots). This could be a breaking behavior change for deployments that intentionally serve from a symlinked directory.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const normalizedFp = resolve(fpPath); | ||
|
|
||
| // Check if the resolved file is outside the allowed directory | ||
| if (!normalizedFp.startsWith(normalizedFolder)) { |
There was a problem hiding this comment.
The startsWith containment check can be bypassed by sibling paths that share the same prefix (e.g., allowed /var/www vs resolved /var/www2/...), so traversal like ../www2/... may still pass this guard. Since this code is a security boundary for static serving, it’s worth tightening to avoid prefix-based false positives.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
