Skip to content

Close the family-nav host check at the slash - #65

Merged
oblomov-dev merged 2 commits into
mainfrom
claude/repos-tests-improvements-ia9cna
Aug 28, 2026
Merged

Close the family-nav host check at the slash#65
oblomov-dev merged 2 commits into
mainfrom
claude/repos-tests-improvements-ia9cna

Conversation

@oblomov-dev

Copy link
Copy Markdown
Member

scripts/check-family-nav.mjs collected "our" links with
h.startsWith(BASE), where BASE is https://abap2ui5.github.io — a prefix
test on a URL that a host name may continue past.
https://abap2ui5.github.io.example.com/samples/ starts with it, so a link to
somebody else entirely would have been collected as one of the three and
compared, in order, against the wanted list. The check whose whole job is to
say which pages the bar links to could not have caught that.

With the trailing slash (https://abap2ui5.github.io/) the host is closed and
the prefix can only match this origin. Every real link already carries it, so
nothing about the current pages changes.

Found by reproducing this organisation's CodeQL configuration locally
(security-extended): it is js/incomplete-url-substring-sanitization, high
severity, twice in this file. Verified gone after the change, with the rest of
the result set unchanged.

The body of this file is shared with abap2UI5/.github/shared/check-family-nav.mjs
and the two sibling sample repositories, and is gated against them — it is
changed in all four or in none. The matching pull requests are open in
samples and samples-controls; the abap2UI5 half follows once these are
merged, because its gate reads the consumers from main.

node scripts/check-family-nav.mjs passes here and in both siblings.


Generated by Claude Code

`startsWith(BASE)` is a prefix test on a URL, and a host name may continue
where the prefix stops: `https://abap2ui5.github.io.example.com/samples/`
starts with `https://abap2ui5.github.io` and was therefore collected as one
of "our" links and compared, in order, against the wanted list. The check
that exists to say which three pages the bar links to would have counted a
link to somebody else as one of them.

With the trailing slash the host is closed and the prefix can only match
this origin. Every real link already carries it, so nothing about the
current pages changes - this is the case the check could not have caught.

The body is shared with abap2UI5/.github/shared/check-family-nav.mjs and the
two sibling repositories; it is changed in all four or in none.
Copilot AI lite review requested due to automatic review settings August 28, 2026 23:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

`A2UI5_URL_RE` was `/github\.com\/abap2UI5\/abap2UI5/`, tested against a whole
`{...}` slice of a "dependencies" array. It matched anywhere in the slice and
it stopped at the repository name without closing it, so
`https://github.com/abap2UI5/abap2UI5-local` - a different repository in the
same organisation, and a plausible thing for an abaplint config to depend on -
was read as the framework and had its "branch" key checked, allowlisted and
reported as if it were.

Anchoring at the `"url"` key and closing at the quote leaves only the
framework. Output is byte-identical on every config in this repository today
(same entry counts, same release, same allowlisted pins); the difference is
the entry that does not exist yet.

CodeQL reports the old form as js/regex/missing-regexp-anchor, high severity.

The file is byte-identical with abap2UI5/.github/shared/check-framework-pin.mjs
and the sibling copy, and is synced from there - changed in all three or in
none.
@oblomov-dev
oblomov-dev merged commit c622700 into main Aug 28, 2026
24 checks passed
@oblomov-dev
oblomov-dev deleted the claude/repos-tests-improvements-ia9cna branch August 28, 2026 23:58
oblomov-dev pushed a commit to abap2UI5/abap2UI5 that referenced this pull request Aug 29, 2026
`startsWith(BASE)` is a prefix test on a URL, and a host name may continue
where the prefix stops: `https://abap2ui5.github.io.example.com/samples/`
starts with `https://abap2ui5.github.io` and was therefore collected as one
of "our" links and compared, in order, against the wanted list. The check
that exists to say which three pages the bar links to would have counted a
link to somebody else as one of them.

With the trailing slash the host is closed and the prefix can only match this
origin. Every real link already carries it, so nothing about the current pages
changes - this is the case the check could not have caught.

This is the two high-severity alerts CodeQL reported against #2679:
js/incomplete-url-substring-sanitization, twice in this file. Reproduced
locally with this repository's own codeql.yaml configuration
(security-extended, same paths), confirmed gone after the change, and the rest
of the result set unchanged.

The body is shared with the three sample repositories and gated against them;
the matching pull requests are abap2UI5/samples#809,
abap2UI5/samples-controls#166 and abap2UI5/samples-stack#65, and they merge
first - check:shared reads the consumers from their main.
oblomov-dev pushed a commit to abap2UI5/abap2UI5 that referenced this pull request Aug 29, 2026
`A2UI5_URL_RE` was `/github\.com\/abap2UI5\/abap2UI5/`, tested against a whole
`{...}` slice of a "dependencies" array. It matched anywhere in the slice and
it stopped at the repository name without closing it, so
`https://github.com/abap2UI5/abap2UI5-local` - a different repository in the
same organisation, and a plausible thing for an abaplint config to depend on -
was read as the framework and had its "branch" key checked, allowlisted and
reported as if it were.

Anchoring at the `"url"` key and closing at the quote leaves only the
framework. Output is byte-identical on every config in both consumers today
(same entry counts, same release, same allowlisted pins); the difference is
the entry that does not exist yet.

CodeQL reports the old form as js/regex/missing-regexp-anchor, high severity,
and stops reporting it after this change.

Synced to abap2UI5/samples#809 and abap2UI5/samples-stack#65, which merge
first.
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.

3 participants