ci: validate landscape.yml on pull requests - #28
Open
thc1006 wants to merge 16 commits into
Open
Conversation
Add a pull_request workflow and a small Python validator for landscape/landscape.yml. The Pages deployment only runs on push to main and copies the YAML without parsing it, so malformed data currently passes review and only fails when the browser loads the map. The validator checks structure, required item fields, the project enum, https URLs, and duplicate names against docs/data-schemas.md. It passes on the current data and can also be run locally. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Replace the earlier PyYAML validator with one built on js-yaml 4.1.0, the same library and version the site loads in the browser. PyYAML silently accepts things js-yaml rejects, most importantly duplicate mapping keys, so a Python-based check could pass data that then breaks the rendered map. Using the site's parser keeps CI and the browser in agreement. The validator also checks the category/subcategory/item structure, required fields, the project enum, https URLs, unexpected fields, and duplicate category, subcategory, or entry names, and rejects an empty landscape. A node:test suite covers the happy path and each failure mode and runs in CI, so the validator cannot be weakened without a test failing. The workflow runs on every pull request (no path filter, so it is safe as a required check) and on push to main, with a timeout, concurrency, and least-privilege permissions. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…idation Bump js-yaml from 4.1.0 to 4.1.1 in both the validator (scripts) and the browser (the CDN script in landscape/static), fixing CVE-2025-64718: in 4.1.0 a `__proto__` merge key can pollute the parsed object's prototype. Under 4.1.0 an item with no own required fields could inherit them via a merge payload and pass validation; a regression test now covers that case. The validator also reads every schema field as an own property (Object.hasOwn / Object.keys) so prototype values can never satisfy a required field, rejects categories with no subcategories and subcategories with no items, and checks for unknown fields at the top, category, and subcategory levels rather than only on items. Move the CI job to Node 22 (Node 20 is end-of-life), install with --ignore-scripts, pin the CDN script with Subresource Integrity, and run the validator in the Pages deploy job so a bad file cannot be published on a direct push to main. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The bundled js-yaml 4.1.1 is affected by two merge-key DoS advisories (GHSA-h67p-54hq-rp68, GHSA-52cp-r559-cp3m); npm audit flags it. Move both the CI dependency and the browser bundle to 4.3.0, which clears both. cdnjs does not host 4.3.0, so the browser now loads it from jsDelivr with a refreshed SRI hash. Beyond the library, YAML aliases resolve to shared references, so a small file with nested aliases expands to N^3 item visits in the validator (and N^3 DOM nodes in the browser) from ~3N lines. The schema never needs anchors, aliases, or merge keys, so reject any reused node, and cap the file size. Gate the subcategories/items reads on Object.hasOwn to match the module's own-property contract. Tests cover each new guard. Align the Pages deploy job with the validation workflow: Node 22 (20 is end-of-life), a job timeout, and persist-credentials: false on checkout. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The file-size cap bounded the input but not what it materializes: a document well under the cap could anchor a large description and reuse it via a scalar alias across many items (the object-alias guard only sees reused objects, not reused primitives), or simply carry one huge description or thousands of items, producing an enormous browser render. Parse with FAILSAFE_SCHEMA and maxDepth, which restrict types to strings, sequences, and mappings and disable merge (`<<`) resolution, so a merge key is rejected as an unknown field. Cap the item count and each text field's length so a reused or oversized scalar is rejected per occurrence. Also reject URLs that carry credentials and normalize names with NFKC before duplicate detection. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…r cost Addresses review feedback on the landscape validator. The browser (landscape/static/app.js) parsed landscape.yml with js-yaml's default schema while the CI validator used FAILSAFE_SCHEMA, so a scalar such as `name: 789` or `description: 2026-01-01` validated as a string but became a number or Date in the browser, where the search code then threw calling .toLowerCase() on it. app.js now parses with the same FAILSAFE_SCHEMA and maxDepth options, and a test guards the parity. Bound the work a small but hostile file can force. The validator caps category and subcategory names (so an over-long name cannot inflate every error message), rejects control and format characters in names, caps the logo length, preflights the item count before the detailed pass, lowers the item cap to 500 (the landscape has a few tens of entries), and caps the number and length of reported errors. app.js bounds the highlight nodes a single field can create and builds the load-error message with textContent instead of innerHTML. Document the enforced limits in docs/data-schemas.md so the validator matches the contributor contract. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The review found two validator holes. `logo` was only length-checked when it was already a string, so a sub-2MB document could set it to a sequence of hundreds of thousands of distinct empty mappings that passed validation and then deployed to every browser. And the error cap only bounded stored output, not the construction of error strings, so an oversized scalar alias in `project` (uncapped), `name`, a URL, or an unknown key was interpolated and normalized before it could be discarded. Add a per-item preflight that checks every schema field's type and length (and the name's control characters) using only the bounded item index, and skip the rest of the item when a field fails, so no oversized or non-string value reaches a location, normalizeKey, the URL parser, or a diagnostic. Cap `project` length; escape and bound the value shown in the project and unknown-field diagnostics; and require the literal `https://` prefix, since the WHATWG parser canonicalizes forms like `https:host`. Fix the schema-doc example, which was missing the top-level `landscape:` key, and state that item names are unique across the whole landscape. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Review feedback, all making the code match its own claims: - Bound YAML parse-error messages too. js-yaml puts the offending token (for example an undefined alias name) into the message, so an oversized token bypassed the MAX_ERROR_LENGTH cap that every other diagnostic respects. - Reword the alias-rejection message and the docs. hasReusedNode rejects reused object/array nodes and cycles, not scalar aliases (which stay within the field and total limits), so "anchors/aliases are rejected" overstated it. - Reject control/format characters in descriptions, not only names, since descriptions are rendered too and the comment already claimed "any displayed string". Fold the type, length, and control-character checks into one pass so the control check runs only on a value already within its length cap. - Document the project length limit (50). Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
renderLandscape read state.currentSearch raw while runFilteringPipeline filtered on the lower-cased, trimmed query, so the two could disagree. A search with leading or trailing spaces filtered items in but highlighted nothing, and a whitespace-only search kept every item while building a regex from the raw spaces for each rendered field. Pass the normalized query into renderLandscape so highlighting matches what was filtered. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Add a container and edge budget to the graph walk so a wide unknown key (for example a large top-level list of empty mappings, which does not count toward the item cap) is rejected once it exceeds 5000 objects instead of being walked in full; the reused-node check is folded into the same pass. Check the file size with statSync before reading it in the CLI so an oversized file is not read into memory first, while validate() keeps its own byte check for unit tests and other callers. Replace the highlighter's String.split with a bounded RegExp.exec walk, so a long value with many matches no longer allocates a full fragment array before the node cap applies, and cap the search input length. Fix the docs schema example, whose repo_url carried an inline "(optional)" the validator rejects as whitespace in a URL; label the repository link "Repository" rather than "GitHub" since GitLab and others are allowed; and correct the js-yaml comment's affected range. Add tests for the object budget and that the docs example validates. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…st the SRI The highlighter capped matches per field but not per render, so a landscape near the 500-item cap could still create a large number of <mark> nodes on one search. Compile the search regex once per render and share a total <mark> budget across every field. Correct the graph-budget wording: js-yaml materializes the parsed document before the walk runs, so the walk stops the validation traversal early, it does not stop the parse. The byte cap is what bounds the parse, so lower it from 2 MB to 512 KB (the data is ~14 KB and the 500-item cap keeps a realistic file well under this). Document the edge budget, and walk mappings with for...in so a wide object hits the edge cap without first allocating its full key array. Add a test that the index.html js-yaml version and SRI match the installed bundle and that app.js keeps maxDepth in sync, so a bump that would break the browser on an SRI mismatch fails CI instead of passing silently. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…ests The default landscape path was resolved against the current working directory, so running the validator from the repo root looked for ../landscape/landscape.yml outside the repo. Resolve it relative to the script instead, so the command works from anywhere. Compare maxDepth between app.js and the validator rather than only asserting app.js has some value, and add a test for the graph edge budget alongside the existing container-budget test. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
upload-pages-artifact packs dist with tar --dereference, so a symlink copied into dist (cp -r brings the repo tree in) would be published as the bytes of its target, for example a runner-local file. A symlink is easy to miss in review, so fail the deploy if any are present rather than leak the target to public Pages. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The filter used toLowerCase().includes() while the highlighter used a case-insensitive RegExp, so for some Unicode inputs (for example a Turkish dotted-I) an item could match the search yet not be highlighted. Build both from one escaped, case-insensitive regex so an item is highlighted exactly when it matched; behavior is unchanged for ASCII. Also reject symlinks in the deployed sources before assembly, not only in dist after cp: whether cp preserves or dereferences a symlink is version-dependent, and a dereferenced one would be a plain file in dist that the dist check misses. The source check is scoped to the copied paths so it does not trip on node_modules symlinks under scripts/. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Build the filter and highlight regexes with the `u` flag (`iu`/`giu`) so both apply Unicode simple case-folding and keep agreeing on what matched: a Kelvin sign now matches `k` and a capital sharp-S matches `ß`, in the filter and the highlight alike. This needs a `u`-safe escape, so escapeRegExp no longer escapes `-` (it is literal outside a character class, and `\-` is an invalid identity escape under `u`); the standard syntax-character set it now escapes is unchanged in meaning for every earlier query. Move the source-symlink guard into scripts/check-no-symlinks.sh and run it, plus `node --check` on the browser script, from the pull-request workflow as well as the deploy. A committed symlink or a syntax error in app.js is now caught before merge, not only when main is deployed. Correct two stale comments: the shared query is trimmed, not lower-cased (case is handled by the flag), and the filter also searches the tier and the URLs, which are not rendered as highlightable text, so a match there filters a card in without a visible mark. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Adds a source-parity check, alongside the existing FAILSAFE_SCHEMA and maxDepth ones, asserting the filter and highlight regexes keep the `u` flag and that escapeRegExp does not escape `-`. This fails if either the Unicode case-folding or the `u`-safe escaping is dropped from app.js. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
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.
What this PR does
Adds
scripts/validate-landscape.mjs, a CI validator forlandscape/landscape.ymlthat parses with the same js-yaml version and options the site loads with (FAILSAFE_SCHEMA,maxDepth), so anything the browser would reject or mis-type fails CI instead of passing review and breaking the rendered map. On top of parsing it checks the category/subcategory/item structure, required fields, theprojectenum, https URLs (no whitespace or credentials), unknown fields, and duplicate names (Unicode-normalized).It also bounds the work a small but hostile file can force, on both sides. The 512 KB byte cap (the data is ~14 KB) is what limits how much js-yaml materializes; on top of that the validator caps field lengths, display-name lengths, the item count (500), the object graph (5000 objects or arrays and 20,000 references, a budget that stops the validation walk early once exceeded, so a wide unknown top-level key cannot slip under the item cap), and the number and size of reported errors, and it rejects reused object or array nodes (YAML aliases or cycles), merge keys, and control or format characters (a bounded scalar alias is allowed). The CLI checks the file size with
statSyncbefore reading it. The browser (app.js) now parses with the same failsafe options, so a scalar likename: 789can no longer validate as a string here yet arrive as a number in the browser and throw on the first search keystroke. It also highlights with a boundedRegExp.execwalk (no fullString.splitarray), compiling the query once per render and sharing a total match budget across every field, using the same trimmed query and case-folding the filter matched with (both build the regex with theuflag, so Unicode folds such as a Kelvin sign matchingkagree between them); it caps the search input length and builds its load-error message withtextContentinstead ofinnerHTML. The enforced limits are documented indocs/data-schemas.md.scripts/validate-landscape.test.mjshas 49node:testcases covering the real landscape, the failure modes, the parser parity (a numeric scalar stays a string;app.jsuses the failsafe schema and keeps its search regexes Unicode-aware), the object-graph container and edge budgets, that the schema example in the docs validates, that the browser js-yaml version and SRI match the installed bundle, and the bounds. The workflow runsnpm ciand the tests with a pinnedjs-yaml(committedpackage-lock.json), Node 22,timeout-minutes, andpersist-credentials: false. It also runsnode --checkonapp.jsand the symlink guard on every pull request, so a syntax error or a committed symlink is caught before merge, not only whenmaindeploys.The symlink guard is a shared
scripts/check-no-symlinks.shthat rejects symlinks in the deployed sources; the deploy runs it before assembly and rechecks the assembleddistafterward.upload-pages-artifactpacks the tree withtar --dereference, so a symlink committed to the repo could otherwise be published as the bytes of its target (for example a runner-local file).Deferred follow-ups
A build-to-JSON step (removing browser-side YAML entirely), a Playwright browser smoke test, splitting the Pages build and deploy into separate jobs,
merge_groupsupport, and full-SHA action pinning are reasonable next steps but are out of scope for this validator PR.