[Audit][Medium] Input.setMouseCapture type safety inconsistency

[MichaelFisher1997]

🔍 Module Scanned

src/engine/input/ (automated audit scan)

📝 Summary

The setMouseCapture function in Input has inconsistent type handling between its public API (which accepts anytype and passes directly to SDL) and the vtable implementation (which properly casts from ?*anyopaque to *c.SDL_Window). This creates a type safety gap where the public API could accept incompatible window types without compile-time verification.

📍 Location

• File: src/engine/input/input.zig:184-187
• Function/Scope: Input.setMouseCapture (public method)
• Secondary Location: src/engine/input/input.zig:301-306 (vtable implementation)

🔴 Severity: Medium

• Medium: Inconsistent API design, potential for misuse, silent type coercion

💥 Impact

The public setMouseCapture method accepts anytype for the window parameter and passes it directly to SDL_SetWindowRelativeMouseMode without explicit type checking. While callers in this codebase consistently pass *c.SDL_Window or properly cast ?*anyopaque, the API design allows passing any pointer type. If an incompatible type is passed, this could cause undefined behavior or a segfault at runtime rather than a compile-time error.

Additionally, the vtable implementation at line 301-306 does proper type casting (@as(*c.SDL_Window, @ptrCast(@alignCast(w)))) with a null check before calling back into self.setMouseCapture, creating an inconsistent pattern where the safer code path goes through the vtable while the direct public API lacks the same safety.

🔎 Evidence

Public API (input.zig:184-187):

pub fn setMouseCapture(self: *Input, window: anytype, captured: bool) void {
    self.mouse_captured = captured;
    _ = c.SDL_SetWindowRelativeMouseMode(window, captured);
}

VTable implementation (input.zig:301-306):

fn impl_setMouseCapture(ptr: *anyopaque, window: ?*anyopaque, captured: bool) void {
    const self: *Input = @ptrCast(@alignCast(ptr));
    if (window) |w| {
        self.setMouseCapture(@as(*c.SDL_Window, @ptrCast(@alignCast(w))), captured);
    }
}

The vtable:

1. Receives window: ?*anyopaque from the interface
2. Performs a null check with if (window) |w|
3. Explicitly casts w from *anyopaque to *c.SDL_Window using @ptrCast(@alignCast())
4. Calls self.setMouseCapture(...) which then passes directly to SDL again

Interface definition (interfaces.zig:34):

setMouseCapture: *const fn (ptr: *anyopaque, window: ?*anyopaque, captured: bool) void,

🛠️ Proposed Fix

Refactor setMouseCapture to ensure consistent type handling. The public API should either:

Option 1 (Recommended): Make the public API accept the correct typed pointer and remove the redundant SDL call from the vtable:

pub fn setMouseCapture(self: *Input, window: *c.SDL_Window, captured: bool) void {
    self.mouse_captured = captured;
    _ = c.SDL_SetWindowRelativeMouseMode(window, captured);
}

And update the vtable implementation to not call back into the public method:

fn impl_setMouseCapture(ptr: *anyopaque, window: ?*anyopaque, captured: bool) void {
    const self: *Input = @ptrCast(@alignCast(ptr));
    if (window) |w| {
        self.mouse_captured = captured;
        _ = c.SDL_SetWindowRelativeMouseMode(@as(*c.SDL_Window, @ptrCast(@alignCast(w))), captured);
    }
}

Option 2: If anytype is intentional for API flexibility, add explicit type validation:

pub fn setMouseCapture(self: *Input, window: anytype, captured: bool) void {
    self.mouse_captured = captured;
    const typed_window = @as(*c.SDL_Window, @ptrCast(@alignCast(window)));
    _ = c.SDL_SetWindowRelativeMouseMode(typed_window, captured);
}

✅ Acceptance Criteria

• The setMouseCapture public API has explicit type signature matching SDL's expectation
• The vtable implementation and public API have consistent type handling
• No redundant calls between vtable implementation and public method
• All existing callers in src/game/map_controller.zig, src/game/screens/*.zig continue to work
• Verified with nix develop --command zig build test

📚 References

• Ziglang documentation on anytype: https://ziglang.org/documentation/master/#Type-System
• Related: src/engine/core/window.zig:6 defines WindowManager.window as *c.SDL_Window
• Callers: src/game/map_controller.zig:38-42, src/game/screens/world.zig:505,510, src/game/screens/home.zig:100

[github-actions]

Comment:

The audit finding is accurate. The type safety inconsistency in setMouseCapture is a valid bug.

Analysis:

• Lines 184-187: Public API accepts anytype, passes directly to SDL without validation
• Lines 301-306: VTable properly casts ?*anyopaque → *c.SDL_Window with null check, then redundantly calls back into the public method
• Callers: Mix of patterns - some pass typed *c.SDL_Window directly (screens), others use explicit @ptrCast(@alignCast(...)) (session, world.zig)

Root cause: The vtable pattern correctly handles the opaque pointer from the interface, but then re-calls the public API which bypasses type safety. Additionally, the public API accepting anytype defeats compile-time verification.

Recommended fix: Option 1 from the report is correct - change the public signature to *c.SDL_Window and have the vtable implementation perform the SDL call directly without re-calling self.setMouseCapture:

// Public API (line 184)
pub fn setMouseCapture(self: *Input, window: *c.SDL_Window, captured: bool) void {
    self.mouse_captured = captured;
    _ = c.SDL_SetWindowRelativeMouseMode(window, captured);
}

// VTable (line 301-306) - no callback to public method
fn impl_setMouseCapture(ptr: *anyopaque, window: ?*anyopaque, captured: bool) void {
    const self: *Input = @ptrCast(@alignCast(ptr));
    if (window) |w| {
        self.mouse_captured = captured;
        _ = c.SDL_SetWindowRelativeMouseMode(@as(*c.SDL_Window, @ptrCast(@alignCast(w))), captured);
    }
}

Scope: Single function in src/engine/input/input.zig. All callers (src/game/map_controller.zig, src/game/screens/*.zig, src/game/session.zig) pass *c.SDL_Window and will work without modification since they already pass the correct type.

opencode session  |  github run