enforce strict relative subpath validation in builders; reject absolu… - #2
Conversation
…te and Windows-style subpaths with actionable errors; document subpath validation; add OS matrix CI Signed-off-by: Prabhu Subramanian <prabhu@appthreat.com>
…cross platforms Signed-off-by: Prabhu Subramanian <prabhu@appthreat.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens subpath handling to prevent absolute/Windows-style subpaths when constructing PURLs from parts (builders/object flows), while keeping parser compatibility via canonicalization. It also documents the behavior and adds CI coverage across OSes.
Changes:
- Enforce strict relative
subpathvalidation inbuild()/Purl.from()/typed builders (reject absolute + Windows absolute forms withE_INVALID_SUBPATH). - Add tests covering new subpath validation and parser canonicalization behavior.
- Document subpath rules in the README and add a GitHub Actions OS matrix job.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
index.js |
Adds stricter subpath validation logic rejecting absolute and Windows absolute inputs. |
test/typed-builders.test.js |
Adds assertions that builder/object flows reject absolute and Windows absolute subpaths. |
test/strict-encoding-edge-cases.test.js |
Adds a compatibility test ensuring parse() canonicalizes absolute-looking raw subpaths. |
README.md |
Documents builder/object subpath validation rules and parser canonicalization behavior. |
.github/workflows/ci.yml |
Adds an OS matrix CI job running the test suite on ubuntu/macos/windows. |
package.json |
Bumps package version to 0.0.2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (subpath.startsWith("\\") || /^[A-Za-z]:[\\/]/.test(subpath)) { | ||
| throw createError( | ||
| "E_INVALID_SUBPATH", | ||
| 'Absolute subpaths are not allowed; use a relative subpath like "a/b" instead of "/a/b"' | ||
| ); |
There was a problem hiding this comment.
The error message used for Windows absolute subpaths is the same as the POSIX case and suggests avoiding "/a/b", which is confusing when the rejected input is like "C:\docs\api" or "\\server\share". Consider tailoring the message based on which absolute form was detected (or using a more generic example that covers both), and de-duplicating the message text between the two checks so they can’t drift.
| - name: Check generated type-rules freshness | ||
| run: node scripts/check-generated-type-rules.mjs | ||
|
|
||
| - name: Run full test suite |
There was a problem hiding this comment.
This OS-matrix job runs pnpm test (i.e., node --test), which will also execute the deterministic fuzz suites with their default case counts/seeds on macOS/Windows. That can significantly increase CI time and flake risk compared to the main ubuntu job where fuzzing is controlled via explicit env vars. Consider running only the focused suites here (or setting much smaller PURL_FUZZ_* env values / skipping fuzz) to keep the matrix fast and stable while still validating OS-specific behavior.
| - name: Run full test suite | |
| - name: Run full test suite | |
| env: | |
| PURL_FUZZ_CASES: "10" | |
| PURL_FUZZ_SEED: "1" |
…te and Windows-style subpaths with actionable errors; document subpath validation; add OS matrix CI