Skip to content

Test/safelink inert rendering coverage#512

Open
Favourejiro wants to merge 2 commits into
Disciplr-Org:mainfrom
Favourejiro:test/safelink-inert-rendering-coverage
Open

Test/safelink inert rendering coverage#512
Favourejiro wants to merge 2 commits into
Disciplr-Org:mainfrom
Favourejiro:test/safelink-inert-rendering-coverage

Conversation

@Favourejiro

Copy link
Copy Markdown
Contributor

closes #365

tighten unit-test coverage for SafeLink's inert rendering and rel/target attribute enforcement across the full
unsafe-scheme matrix.

While extending the tests, the issue's required assertions surfaced two real link-safety gaps that had to be fixed for the assertions to
hold. Both are small, security-positive changes.

Changes

src/components/SafeLink.tsx — enforce safety attributes

Previously {...props} was spread after target/rel, so a caller could override them (e.g. rel="" or target="_self"),
silently defeating tab-nabbing protection. The spread now comes before the enforced attributes, so target="_blank" and
rel="noopener noreferrer" always win while non-conflicting props (e.g. className) still pass through.

src/utils/url.ts — reject userinfo-bearing URLs

normalizeEvidenceUrl accepted URLs like https://trusted.com@evil.com because the protocol was https:. Userinfo
(username/password) is now rejected — it's a common credential/phishing vector that hides the real host.

src/components/__tests__/SafeLink.test.tsx — expanded coverage

  • Unsafe-scheme matrix (test.each): javascript:, data:, empty, whitespace-only, missing-scheme, and userinfo-bearing URLs all
    render the inert [Invalid Link] span with the Rejected URL: … title.
  • Safe URLs: http/https (incl. query strings) render an <a> with target="_blank" and rel="noopener noreferrer".
  • Conflicting-props case: caller-supplied target/rel cannot override the enforced values.
  • Prop forwarding: non-conflicting props such as className are forwarded.
  • Children: rendered in the safe branch; not rendered in the unsafe branch.

Testing

npm run test -- src/components/tests/SafeLink.test.tsx src/utils/tests/url.test.ts

  • SafeLink.test.tsx — 14 passing
  • url.test.ts — 10 passing
  • Consumers (ValidationDetail, ConfirmationModal, EvidenceUpload) — 49 passing, no regressions in link safety.

Add RTL cases for trailing-slash path differences in NavLink's
startsWith-based active logic (issue Disciplr-Org#321): a trailing slash on `to`
that the current path lacks stays inactive, and a current path that
appends a trailing slash to `to` is active.
Tighten SafeLink coverage and the link-safety it relies on (issue Disciplr-Org#365):

- Add a full unsafe-scheme matrix (javascript:, data:, empty, whitespace,
  missing scheme, userinfo-bearing) asserting the inert [Invalid Link] span
  and its rejected-URL title, plus children-rendering in both branches.
- Enforce target="_blank" and rel="noopener noreferrer" against caller
  overrides by spreading caller props before the safety attributes.
- Reject userinfo-bearing URLs (e.g. https://trusted.com@evil.com) in
  normalizeEvidenceUrl, closing a credential/phishing vector.
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.

Add unit tests for the SafeLink inert rendering and attribute enforcement

1 participant