Repository navigation
fix/docs: disabled nav link semantics + SECURITY.md merge-corruption (#610, #611) - #616
Merged
Austinaminu2 merged 2 commits intoAug 24, 2026
Merged
Conversation
…lowwStar#610) Base UI's `disabled` prop on a `Button asChild` wrapping a `<Link>` doesn't give the resulting <a> real disabled semantics: - `:disabled` CSS never matches an anchor (only real form controls), so the intended `disabled:opacity-50 disabled:pointer-events-none` styling never actually applies in a real browser. - No `aria-disabled` is set, so assistive tech gets no signal. - The element keeps `tabindex="0"` and stays in the tab order. - `href` remains live, so middle-click/ctrl+click can still open the route in a new tab, bypassing the `auxclick`-unaware disabled guard. Verified empirically (see the new navbar test) that a plain left-click IS correctly blocked today — Base UI's injected onClick calls preventDefault, and next/link's own handler checks e.defaultPrevented before navigating — so the link is not actually click-navigable as the issue feared. The remaining problems above are real, though, and the cleanest fix for all of them is to stop rendering an anchor at all when disabled: render a real `<button disabled>` (proper :disabled styling, automatically out of the tab order, no href to open) instead of a Button-wrapping-Link when networkMismatch is true. Audited the other two Button asChild + Link usages named in the issue (components/landing/hero.tsx, components/streams/empty-state.tsx) — neither sets a `disabled` prop, so this pattern doesn't occur there.
The file contained two full drafts of itself concatenated together
(two Scope sections, two reporting-process sections, two Response
Timeline tables, two severity tables, two Safe Harbor sections, two
Bug Bounty sections). Reconciled into one internally-consistent policy
rather than picking either half wholesale:
- Scope: merged the broader FlowStar-wide language with the second
draft's more specific examples (XSS/CSRF/wallet-key exposure,
supply-chain/secret exposure). Kept `contracts/` (not the narrower
`contracts/streaming/`) as the scope path since it's forward-compatible
if more contract packages are added later.
- Response Timeline: the two drafts agreed on every window except Low
severity ("Best effort, based on project priorities" vs "Next
scheduled release") — merged to "Best effort — typically bundled
into the next scheduled release," which keeps both meanings instead
of dropping one.
- Severity Guide: merged both example sets per severity level (e.g.
added the second draft's "DoS affecting individual users" to Medium,
and "unauthorized access to user data" to High) into one table.
- Safe Harbor: merged the first draft's researcher-facing checklist
with the second draft's more explicit maintainer commitments ("we
will not refer you to law enforcement").
- Bug Bounty: kept the first draft's more accurate framing — this repo
runs paid third-party campaigns (see the "Maybe Rewarded"/"GrantFox
OSS" issue labels), so a flat "no bounty program" would be wrong.
Flagging for maintainer review rather than silently resolving: one
draft's "Reporting a Vulnerability" section included an email fallback
("email the maintainers at the address listed on the GitHub profile")
for urgent critical issues. I did not carry that into the merged doc —
"the address listed on the GitHub profile" isn't a concrete, verifiable
contact, and publishing an unclear channel in a security policy seems
worse than omitting it. If maintainers want a backup channel to GitHub
Security Advisories, it needs a real, confirmed address.
|
@SYMBAxx Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This branch had an error being deployed
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.
Bundles four assigned issues; two needed real changes, two turned out to already be resolved on
main.closes #610 — Audit semantics wrapping disabled navigation links
Verified empirically (new test in
__tests__/components/navbar.test.tsx, using a mock router + realnext/link) that a plain left-click on the "New stream" link during a network mismatch is actually blocked today: Base UI'sButton asChildinjects anonClickthat callspreventDefault()whendisabled, andnext/link's own handler checkse.defaultPreventedbefore navigating.That said,
disabledon aButton asChildwrapping an<a>doesn't give the anchor real disabled semantics, and those gaps are real::disabledCSS never matches an anchor (only real form controls), so thedisabled:opacity-50 disabled:pointer-events-noneclasses never actually apply — the control doesn't visually look disabled in a real browser.aria-disabledis set — assistive tech gets no signal.tabindex="0"keeps it in the tab order.hrefstays live, so middle-click (auxclick, which Base UI doesn't intercept) can still open the route in a new tab.Fixed by conditionally rendering a real
<button disabled>(noasChild, nohref) whennetworkMismatchis true, instead of a Button-wrapping-Link with adisabledprop. This fixes all four gaps at once — no anchor exists at all while disabled.Audited the other two
Button asChild+Linkusages named in the issue (components/landing/hero.tsx,components/streams/empty-state.tsx) — neither sets adisabledprop, so this pattern doesn't occur there. No changes needed.closes #611 — SECURITY.md merge-corruption duplication
The file had two full drafts concatenated together. Reconciled into one internally-consistent policy — see the commit message for the specific disagreements (Low-severity SLA wording, severity-table examples, Safe Harbor commitments, Scope path) and how each was merged rather than one side being dropped.
Flagging for maintainer review, not silently resolved: one draft mentioned an email fallback ("email the maintainers at the address listed on the GitHub profile") for urgent critical issues. I left that out of the merged doc — "the address listed on the GitHub profile" isn't a concrete, verifiable contact, and publishing a vague channel in a security policy seemed worse than omitting it. If you want a backup channel to GitHub Security Advisories, it needs a real address.
closes #612 — Duplicated Troubleshooting section in CONTRIBUTING.md
Already resolved —
16d8cd3("docs: deduplicate CONTRIBUTING.md sections from unresolved merge", closes #244) fixed this previously. Currentmainhas exactly one## Troubleshootingsection. No changes made; flagging so #612 can be closed as a duplicate of the already-merged fix.closes #613 — Wire Playwright e2e tests into CI
Already resolved —
0adc415("ci: run Playwright e2e tests in CI") added ane2ejob to.github/workflows/ci.ymlthat installs Playwright browsers and runsnpx playwright test. No changes made; flagging so #613 can be closed.Test plan
npm run test— 609/609 passing, coverage thresholds met__tests__/components/navbar.test.tsxcovers both the network-mismatch (disabled button, no link) and normal (real link) casesSECURITY.mdagainst both original drafts to confirm no unique detail was silently dropped