Skip to content

fix(validate): the scan missed four kinds of file, and vouched for prose - #113

Closed
pitimon wants to merge 1 commit into
fix/110-comment-and-pinned-waiversfrom
fix/110-scan-scope-and-metadata
Closed

fix(validate): the scan missed four kinds of file, and vouched for prose#113
pitimon wants to merge 1 commit into
fix/110-comment-and-pinned-waiversfrom
fix/110-scan-scope-and-metadata

Conversation

@pitimon

@pitimon pitimon commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #110 items 2, 6 and 1. Stacked on #111 — review that first, or read this diff two-dot against fix/110-comment-and-pinned-waivers.

Item 2 — the walker saw two directories and one extension set

Added dashboard/*.html (the served shell — a <link rel=preconnect>, font CDN or <script src> here is a browser request), .css (skipped by extension while the walker was already standing in that directory), i18n .json, dashboard/vite.config.js, and scripts/.

That surfaced eight things nothing had ever mentioned. Each was read in source before being classified — declaring without looking would just be a waiver nobody reviewed:

Found What it actually is Declared as
scripts/build-pricing-seed.cjs:10 really fetches the LiteLLM price list request_from
src/lib/pricing/seed-snapshot.json:1 the seed recording its source URL data_from pin
dashboard/vite.config.js:1188 dev server proxying /proxy/ipcheck request_from
dashboard/index.html ×5 footer links + JSON-LD sameAs to the upstream repo link_from pins
www.tokentracker.cc JSON-LD url/logo/breadcrumbs — search-engine metadata, not fetched new entry, data_from
www.npmjs.com JSON-LD downloadUrl. Distinct from registry.npmjs.org, which npx really contacts new entry, data_from
schema.org the JSON-LD @context identifier, same class as the www.w3.org namespace ignored_hosts
example.invalid the reserved TLD that never resolves, used as a dead endpoint in acceptance scripts ignored_hosts

Item 6 — the inventory's prose was unchecked

user_data: true with readme: false passed. So did a wrong from, and purpose text describing something the code does not do. The fields a reader trusts most were the ones nothing verified.

Types and required values are now checked, and from is checked against where the host is actually reached — a host only ever requested from src/ cannot honestly be browser.

Proved non-vacuous against five deliberately-wrong entries before committing, with a valid control that stays clean.

Item 1 — unresolvable authorities were dropped

The issue's three examples — bracketed IPv6, a Cyrillic lookalike, a single-label intranet host — all resolve now. They predate deferring to new URL(). What is left is interpolation that pins nothing, which is ordinary code, which is exactly why the flag-flip version had to be reverted inside #109.

So each is declared with a why, matched on file + authority text, and a new one fails. 15 declared across 8 files: the local bind and Host-header parsing, git-remote normalisation into project_ref, the user's own HTTPS_PROXY, dev-server proxying, and this scanner's own probe strings. A declaration that no longer matches the code is reported as stale, so the list cannot outlive what it excuses.

Test plan

  • ci:local exit 0 — 903 root tests (+12 over fix(validate): trailing comments read as requests; waivers covered whole files #111), 256 dashboard
  • validate:outbound green: 26 declared hosts
  • 19 new tests: the shell, CSS url(), scripts/, the dev config; five metadata violations plus a valid control; a new unresolvable authority fails, a declared one passes, a stale declaration is reported
  • Two existing tests updated where the change inverts their expectation — an unpinnable authority is now reported as unresolvable rather than silently dropped. Both still assert no host is invented, which was their original point.
  • checkOutbound (46) and collectHosts (21) back under the 50-line rule via scanLine (39), checkUnresolved (30), checkInventoryMetadata (29). File 640 lines.

Remaining in #110

Item 5 (URL composition — following expressions rather than scanning literals) and item 7 (avatar body cap, which is PR #112).

Closes #110 items 2, 6 and 1. Stacked on #111.

ITEM 2 — the walker saw two directories and one extension set.

Added: dashboard/*.html (the served shell — a preconnect, font CDN or script
src here is a browser request), .css (skipped by EXTENSION while the walker was
already standing in that directory), i18n .json, dashboard/vite.config.js, and
scripts/. Maintainer-run is a caveat for the purpose field, not a reason to be
invisible.

That surfaced eight things nothing had ever mentioned, each read in source
before being classified:

  scripts/build-pricing-seed.cjs   really fetches the LiteLLM list -> request_from
  seed-snapshot.json               records that URL as a value      -> data_from
  dashboard/vite.config.js         proxies /proxy/ipcheck           -> request_from
  dashboard/index.html             footer links to the upstream repo -> link_from
  www.tokentracker.cc              JSON-LD metadata, not fetched     -> new entry
  www.npmjs.com                    same block                        -> new entry
  schema.org                       JSON-LD @context identifier       -> ignored
  example.invalid                  reserved never-resolving TLD      -> ignored

ITEM 6 — the inventory's prose was unchecked.

`user_data: true` with `readme: false` passed; so did a wrong `from` and purpose
text describing something the code does not do. The fields a reader trusts most
were the ones nothing verified. Types and required values are now checked, and
`from` is checked against WHERE THE HOST IS ACTUALLY REACHED — a host only ever
requested from src/ cannot honestly be `browser`.

ITEM 1 — unresolvable authorities were dropped.

The issue's three examples (bracketed IPv6, a Cyrillic lookalike, a single-label
intranet host) all resolve now; they predate deferring to new URL(). What is
left is interpolation that pins nothing, which is ordinary code — which is why
the flag-flip version had to be reverted inside #109.

So each is declared with a why, matched on file + authority text, and a new one
fails. 15 declared across 8 files: the local bind and Host-header parsing, git
remote normalisation into project_ref, the user's own HTTPS_PROXY, dev-server
proxying, and this scanner's own probe strings.

Also: checkOutbound and collectHosts were both over the 50-line rule; split into
scanLine (39), checkUnresolved (30) and checkInventoryMetadata (29), leaving
them at 46 and 21. File is 640 lines.

19 new tests. The metadata check was proved non-vacuous against five
deliberately-wrong entries before being committed, with a valid control.

ci:local exit 0: 903 root tests, 256 dashboard.
@pitimon
pitimon deleted the branch fix/110-comment-and-pinned-waivers July 25, 2026 21:53
@pitimon pitimon closed this Jul 25, 2026
@pitimon

pitimon commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #114. This PR was auto-closed when its base branch fix/110-comment-and-pinned-waivers was deleted on merging #111, and GitHub will not reopen or retarget a PR whose base is gone. #114 is the same commit cherry-picked onto main, so it also gets real CI — which this one never had, since the workflow only triggers on pull_request: branches: [main].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant