[Audit][Medium] InputBinding.eql() treats .key and .key_alt as equivalent, breaking distinct binding behavior

[MichaelFisher1997]

🔍 Module Scanned

(automated audit scan)

📝 Summary

The function considers and variants with the same underlying key as equivalent, even though they represent distinct bindings. This causes the input mapper to treat primary and alternate bindings incorrectly when using with the alternate key variant.

📍 Location

• File:
• Function/Scope:

🔴 Severity: Medium

• Critical: Crashes, data corruption, security vulnerabilities, GPU device loss
• High: Memory leaks, race conditions, incorrect rendering, broken features
• Medium: Performance degradation, missing error handling, suboptimal patterns
• Low: Code style, dead code, minor improvements

💥 Impact

When a user binds an action to (or the system uses internally), will incorrectly return when compared against . This means the input mapper's , , and methods compare bindings using with and treat them as the same physical key. This breaks the expected behavior when users want distinct bindings for primary vs alternate inputs—the alternate binding () would incorrectly respond to the primary binding () query.

🔎 Evidence

The same pattern exists in :

The union explicitly defines and as separate variants for a reason—users should be able to bind an action to both W (primary) and Up Arrow (alternate). But deflates this distinction.

🛠️ Proposed Fix

1. Fix to distinguish from : The function should return only when both and use the same variant with the same key value:

2. Add tests for that verify does NOT equal :

3. Consider whether should be queried separately in //. If so, the vtable lookup could map to and to (which currently does not exist—would need to add to ). This is a larger change; the minimal fix is correcting .

✅ Acceptance Criteria

• returns when comparing against
• New unit tests verify and are treated as distinct
• No regression in existing tests

📚 References

• Issue [Audit][Medium] InputSettings.saveFromMapper silently swallows errors from persistence.save #723 () uses this same module
• for context on how bindings are checked
• Zig enum handling:

[github-actions]

Bug Classification: Valid Bug

Analysis:

The issue is confirmed in modules/engine-input/src/interfaces.zig:77-100. The eql function explicitly collapses .key and .key_alt variants:

const self_key = switch (self) {
    .key, .key_alt => |k| k,  // Both treated as same
    else => null,
};

This means InputBinding{ .key = .w }.eql(InputBinding{ .key_alt = .w }) returns true when it should return false.

Root Cause: The switch on lines 79 and 83 uses a shared code path for .key and .key_alt, discarding the variant tag and only comparing the underlying Key value.

Existing Tests Assert Wrong Behavior:

• src/game/input_mapper_tests.zig:58-63: Test named "key vs key_alt same key" expects them equal — this test documents the bug, not correct behavior
• modules/game-core/src/input_mapper.zig:266-274: Same issue

Impact: isBindingStateActive/isBindingStatePressed/isBindingStateReleased in input_mapper.zig:141-159 use eql to match bindings, so querying .key = .w would incorrectly match .key_alt = .w bindings.

Fix Direction:

1. Modify eql to compare variant tags before comparing key values — e.g., check std.mem.eql(u8, @tagName(self), @tagName(other)) or use explicit variant comparisons
2. Update/remove tests at lines 58-63 and 266-274 that assert the buggy equality behavior
3. Add tests verifying .key = .w does NOT equal .key_alt = .w

opencode session  |  github run