Skip to content

Commit 2584ee2

Browse files
habdelraclaude
andcommitted
Harden "Open anyway" guard and overlay-height clamp
Address review feedback on the broken-link overlay: - "Open anyway" now only offers to navigate http(s) references. The CTA forwards the reference into viewCard, so non-http protocols (javascript:, data:, …) are rejected — the same safety posture the plain-text URL display already keeps. Adds a parseHttpUrl helper shared by canOpen and openAnyway. - The room-based height clamp no longer collapses the overlay to 0px when a card is too short to fit it (trigger pinned near both edges). It floors at a usable height so the panel stays reachable, overflowing rather than vanishing — the accepted fallback for very small cards. Adds a regression test that "Open anyway" is withheld for a non-http(s) reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent de92930 commit 2584ee2

2 files changed

Lines changed: 68 additions & 26 deletions

File tree

packages/base/default-templates/broken-link-template.gts

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,22 @@ interface NormalizedAdditionalError {
3434
stack?: string;
3535
}
3636

37+
// Only http(s) references are navigable. The brokenUrl is a card reference
38+
// from trusted serialization, but a corrupted realm could ship a non-http
39+
// value; "Open anyway" forwards it into viewCard, so reject other protocols
40+
// (`javascript:`, `data:`, …) — the same reasoning that keeps the URL display
41+
// plain text rather than a link.
42+
function parseHttpUrl(url: string): URL | null {
43+
try {
44+
let parsed = new URL(url);
45+
return parsed.protocol === 'http:' || parsed.protocol === 'https:'
46+
? parsed
47+
: null;
48+
} catch {
49+
return null;
50+
}
51+
}
52+
3753
// Solid amber warning triangle with a black "!" — a two-tone svg the
3854
// single-colour boxel icon can't express. Shared by the reveal trigger and the
3955
// overlay title; `@size` sets both svg dimensions so each caller can match its
@@ -145,29 +161,16 @@ export default class BrokenLinkTemplate extends GlimmerComponent<{
145161
// "Open anyway" navigates to the broken reference even though it failed to
146162
// load — the host's viewCard decides the destination per submode (a stack
147163
// visit in interact, a code-editor jump in code). Hidden when no viewCard is
148-
// wired (e.g. a context that can't navigate) or the reference can't be parsed
149-
// into a URL.
164+
// wired (e.g. a context that can't navigate) or the reference isn't a
165+
// navigable http(s) URL.
150166
private get canOpen(): boolean {
151-
if (!this.args.viewCard) {
152-
return false;
153-
}
154-
try {
155-
new URL(this.args.brokenUrl);
156-
return true;
157-
} catch {
158-
return false;
159-
}
167+
return !!this.args.viewCard && parseHttpUrl(this.args.brokenUrl) !== null;
160168
}
161169

162170
private openAnyway = () => {
163171
let { viewCard, brokenUrl } = this.args;
164-
if (!viewCard) {
165-
return;
166-
}
167-
let url: URL;
168-
try {
169-
url = new URL(brokenUrl);
170-
} catch {
172+
let url = parseHttpUrl(brokenUrl);
173+
if (!viewCard || !url) {
171174
return;
172175
}
173176
viewCard(url);
@@ -289,16 +292,15 @@ export default class BrokenLinkTemplate extends GlimmerComponent<{
289292
this.tipCorner =
290293
`${above ? 'b' : 't'}${extendLeft ? 'r' : 'l'}` as TipCorner;
291294

292-
// Clamp the panel to the room actually available on the side it opens, so a
293-
// tall error never spills past the card boundary — it scrolls inside
294-
// instead. 600px stays the design ceiling; the 155px floor only relaxes when
295-
// even that wouldn't fit, so the floor never forces a clip.
295+
// Clamp the panel to the room available on the side it opens so a tall error
296+
// scrolls inside the card instead of spilling past its boundary. 600px is
297+
// the design ceiling; a small floor keeps the panel usable and never
298+
// collapses it to nothing when the card is too short to fit it — it may then
299+
// overflow, the accepted fallback for very small cards.
296300
let roomChosen = above ? roomAbove : roomBelow;
297-
let available = Math.max(0, roomChosen - gap - edge);
298-
let maxH = Math.min(600, available);
299-
let minH = Math.min(155, maxH);
301+
let maxH = Math.min(600, Math.max(roomChosen - gap - edge, 96));
300302
overlay.style.setProperty('--bl-max-h', `${maxH}px`);
301-
overlay.style.setProperty('--bl-min-h', `${minH}px`);
303+
overlay.style.setProperty('--bl-min-h', `${Math.min(155, maxH)}px`);
302304
};
303305

304306
// Re-measure the corner if the layout shifts while the overlay is open.

packages/host/tests/integration/components/linksto-broken-link-placeholder-test.gts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,46 @@ module(
240240
);
241241
});
242242

243+
test('"Open anyway" is withheld for a non-http(s) reference', async function (assert) {
244+
await setupRealm();
245+
provideConsumeContext(CardCrudFunctionsContextName, {
246+
createCard: () => {},
247+
saveCard: () => {},
248+
editCard: () => {},
249+
deleteCard: async () => {},
250+
viewCard: () => {},
251+
});
252+
253+
// A corrupted realm could ship a non-http reference; the placeholder still
254+
// renders it as text, but the navigate affordance must never forward a
255+
// javascript:/data: URL into viewCard.
256+
let person = await createPerson({});
257+
getDataBucket(person).set('pet', {
258+
type: 'link-error',
259+
reference: 'javascript:alert(1)',
260+
errorDoc: {
261+
status: 500,
262+
title: 'Internal Server Error',
263+
message: 'boom',
264+
additionalErrors: null,
265+
} satisfies SerializedError,
266+
});
267+
268+
await renderCard(loader, person, 'isolated');
269+
await waitFor('[data-test-broken-link-template]');
270+
271+
let embedded = `[data-test-slot='embedded']`;
272+
await click(`${embedded} [data-test-broken-link-reveal]`);
273+
assert
274+
.dom(`${embedded} [data-test-broken-link-url]`)
275+
.hasText('javascript:alert(1)', 'the reference is still shown as text');
276+
assert
277+
.dom(`${embedded} [data-test-broken-link-open-anyway]`)
278+
.doesNotExist(
279+
'no "Open anyway" affordance for a non-navigable reference',
280+
);
281+
});
282+
243283
test('a link-error sentinel renders the error placeholder with its message', async function (assert) {
244284
await setupRealm();
245285
let person = await createPerson({});

0 commit comments

Comments
 (0)