fix(reader): prevent use-after-free when rendering nested list items

[forketyfork]

Solution

Pressing Cmd+R on a session whose scrollback contained a nested markdown list crashed Architect 0.65.3 with EXC_BAD_ACCESS. The crash report pointed at ui.components.markdown_renderer.buildLines + 3516, called from ReaderOverlayComponent.rebuildLines and ultimately from refreshFromSession.

The culprit lived in the .list_item branch of buildLines:

if (indent_spaces > 0) {
    const spaces = try allocator.alloc(u8, indent_spaces);
    @memset(spaces, ' ');
    defer allocator.free(spaces);                 // scoped to the inner if-block
    try run_inputs.append(allocator, .{ .text = spaces, ... });
}
// spaces is freed here, but run_inputs[0].text still points at it
...
try appendWrappedLines(allocator, &lines, run_inputs.items, ...);

The defer ran when the inner if exited, before appendWrappedLines iterated run.text byte by byte. The crash registers line up exactly with that loop: LDP x24, x25, [sp, #56] then LDRB w23, [x26] reading a freed 2-byte slice (indent_level=1 → indent_spaces=2). It usually went unnoticed because the freed slot kept the original space bytes for a while, but on a long-running process the page eventually got unmapped.

While in the same block, the ordered-list marker buffer (var marker_buf: [32]u8 = undefined) was declared inside an else if branch and its address handed off through run_inputs. That is undefined behaviour even when it happens to work today, so it moves up to the case scope as part of the fix.

The replacement uses a module-level static indent_padding array of 24 spaces (max_list_indent_level * 2). The parser already caps indent_level at 12, and RunInput.text is borrowed (the eventual RenderRun is duplicated downstream in appendWrappedLines), so a const slice into static data is both safe and avoids the allocation entirely.

Verification

A regression test wraps std.testing.allocator in a PoisonAllocator that fills freed memory with 0xAA before delegating to the backing allocator. The new test (renderer handles nested list items without use-after-free) feeds " - nested item\n" through parser.parse and buildLines. Against the previous code it segfaults at exactly the line from the production crash; against the fix all 24 markdown renderer and parser tests pass.

zig build, zig build test, just lint, and zig fmt --check are all green.

Test plan

• Open Architect, run a shell, and produce output containing nested markdown list items (for example, paste a block such as cat <<EOF\n- top level\n - nested item\n - deeper item\nEOF).
• Press Cmd+R to open the reader overlay; confirm the app does not crash and the nested items render with bullet markers.
• Close (Cmd+R or Esc) and reopen the reader several times to confirm repeated invocations stay stable.

Hotfix path: no tracking issue exists for this crash yet; linkage will be added during follow-up cleanup if a report comes in.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

Fixes a crash in the markdown reader overlay by removing a use-after-free in list item indentation/marker handling within the markdown renderer.

Changes:

• Replace per-list-item heap allocation for indentation padding with a safe static padding slice, avoiding dangling pointers during wrapping.
• Move the ordered-list marker buffer to a broader scope to avoid passing references to stack data with an insufficient lifetime.
• Add a regression test using a PoisonAllocator to catch use-after-free behavior when rendering nested list items.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.