Skip to content

Commit 693c488

Browse files
committed
Fail closed when ask overlay accept is not id-keyed
1 parent 38cc8ba commit 693c488

3 files changed

Lines changed: 117 additions & 1 deletion

File tree

src/tui/gate-wire.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,74 @@ describe("wireGates", () => {
458458
});
459459
});
460460

461+
test("sequential permission.gate asks paint B's labels and id-scoped values, not A's", async () => {
462+
await withTestRenderer(async (h) => {
463+
const shell = createAppShell(h.renderer, {
464+
terminal: { columns: 80, rows: 24 },
465+
run: "idle",
466+
});
467+
const emitter = new EventEmitter();
468+
let resolvedA: unknown;
469+
let resolvedB: unknown;
470+
try {
471+
const dispose = wireGates(emitter, shell);
472+
emitter.emit("permission.gate", {
473+
request: baseRequest({
474+
id: "req-a",
475+
subject: "git status",
476+
scopes: [{ id: "scope-a", label: "Allow git A", pattern: "git A*" }],
477+
}),
478+
resolve: (outcome: unknown) => {
479+
resolvedA = outcome;
480+
},
481+
});
482+
expect(shell.overlayKind).toBe("permissions");
483+
expect(shell.overlayList?.select.options.map((option) => option.name)).toEqual([
484+
"Reject",
485+
"Accept once",
486+
"Allow git A",
487+
]);
488+
489+
closeInsetOverlay(shell);
490+
expect(resolvedA).toEqual({ allow: false });
491+
expect(shell.overlayList).toBeNull();
492+
493+
emitter.emit("permission.gate", {
494+
request: baseRequest({
495+
id: "req-b",
496+
subject: "git push",
497+
scopes: [{ id: "scope-b", label: "Allow git B", pattern: "git B*" }],
498+
}),
499+
resolve: (outcome: unknown) => {
500+
resolvedB = outcome;
501+
},
502+
});
503+
expect(shell.overlayKind).toBe("permissions");
504+
const painted = shell.overlayList?.select.options ?? [];
505+
expect(painted.map((option) => option.name)).toEqual([
506+
"Reject",
507+
"Accept once",
508+
"Allow git B",
509+
]);
510+
expect(painted.map((option) => option.value)).toEqual([
511+
`req-b:${PERMISSION_DENY_ID}`,
512+
`req-b:${PERMISSION_ONCE_ID}`,
513+
"req-b:scope-b",
514+
]);
515+
expect(painted.map((option) => option.value)).not.toContain(`req-a:${PERMISSION_DENY_ID}`);
516+
expect(painted.map((option) => option.value)).not.toContain(`req-a:${PERMISSION_ONCE_ID}`);
517+
expect(painted.map((option) => option.value)).not.toContain("req-a:scope-a");
518+
519+
acceptOverlaySelection(shell);
520+
expect(resolvedB).toEqual({ allow: false });
521+
522+
dispose();
523+
} finally {
524+
shell.dispose();
525+
}
526+
});
527+
});
528+
461529
test("operator.gate without id cancels without opening", async () => {
462530
await withTestRenderer(async (h) => {
463531
const shell = createAppShell(h.renderer, {

src/tui/overlays.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,46 @@ describe("overlay accept callbacks", () => {
321321
);
322322
});
323323

324+
test("gate accept with a painted value missing from live itemIds denies instead of remapping by index", async () => {
325+
await withTestRenderer(
326+
async (h) => {
327+
const shell = createAppShell(h.renderer, {
328+
terminal: { columns: 80, rows: 24 },
329+
wireKeys: false,
330+
});
331+
try {
332+
const accepted: OverlaySelection[] = [];
333+
let cancelled = 0;
334+
openPermissionsOverlay(shell, {
335+
items: ["Reject", "Accept once"],
336+
itemIds: ["req-b:__deny__", "req-b:__once__"],
337+
isGate: true,
338+
echoChoice: false,
339+
onAccept: (s) => accepted.push(s),
340+
onCancel: () => {
341+
cancelled += 1;
342+
},
343+
});
344+
moveOverlaySelection(shell, 1);
345+
const list = shell.overlayList;
346+
if (!list) throw new Error("expected an open overlay list");
347+
list.select.options = [
348+
{ name: "Reject", description: "", value: "req-a:__deny__" },
349+
{ name: "Accept once", description: "", value: "req-a:__once__" },
350+
];
351+
list.select.setSelectedIndex(1);
352+
acceptOverlaySelection(shell);
353+
expect(accepted).toEqual([]);
354+
expect(cancelled).toBe(1);
355+
expect(shell.overlayList).toBeNull();
356+
} finally {
357+
shell.dispose();
358+
}
359+
},
360+
{ width: 80, height: 24 },
361+
);
362+
});
363+
324364
test("sequential operator opens paint B's labels and ids, not A's", async () => {
325365
await withTestRenderer(
326366
async (h) => {

src/tui/shell/overlay-host.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,15 @@ export function acceptOverlaySelection(shell: AppShell): void {
776776

777777
const painted = shell.overlayList.select.getSelectedOption()?.value;
778778
const itemIds = bag?.primaryBindings.itemIds ?? [];
779-
const id = typeof painted === "string" && itemIds.includes(painted) ? painted : itemIds[idx];
779+
const idKeyed = typeof painted === "string" && itemIds.includes(painted);
780+
// Gate accept is id-keyed. A painted Select value missing from the live
781+
// itemIds is a stale or mismatched row — remapping via index would bind
782+
// Enter to the new question's same-index choice. Fail closed instead.
783+
if (bag?.primaryBindings.isGate === true && !idKeyed) {
784+
closeInsetOverlay(shell);
785+
return;
786+
}
787+
const id = idKeyed ? painted : itemIds[idx];
780788
// Type-to-filter plants "(no matches)" with an empty-id sentinel. Stay open.
781789
if (id === "") return;
782790
const value = bag?.primaryBindings.itemValues[idx];

0 commit comments

Comments
 (0)