Skip to content

fix: reject off-board and fractional move coordinates - #36

Merged
isamu merged 1 commit into
mainfrom
fix/35-move-position-bounds
Oct 21, 2026
Merged

isamu merged 1 commit into
mainfrom
fix/35-move-position-bounds

Conversation

@isamu

@isamu isamu commented Oct 21, 2026

Copy link
Copy Markdown
Member

Summary

Closes #35.

  • isLegalMove read board[row][col] before checking the position. A negative or fractional row therefore threw a TypeError and failed the whole tool call.
  • The new src/core/position.ts adds isOnBoard(row, col, size), which is true only when both are whole numbers inside the board. isInBounds delegates to it, and isLegalMove checks it before reading the board, so these moves now get the existing invalid-move response.
  • This is also the unchecked index CodeQL reports as js/prototype-polluting-assignment (alerts 1–3). A non-number row such as "__proto__" from a JavaScript caller is now rejected before any assignment.
  • Adds yarn test (tsx --test tests/*.test.ts, the setup the other plugins use) with tests/position.test.ts, and runs it in CI.

Items to Confirm

  • Whether the CodeQL alerts close once this lands on main. CodeQL may not treat Number.isInteger as a sanitizer. The crash itself is fixed either way, as yarn test shows.
  • Tests are not part of yarn typecheck. That matches the other plugins; adding them would need @types/node.
  • Validating a caller-supplied board (wrong size or shape) is out of scope.

Verification

  • yarn format, yarn lint, yarn build, yarn typecheck and yarn test pass.
  • With src/core/logic.ts restored to main, the off-board / fractional test goes red.

User Prompt

  • Inspect the other plugin repos the same way as GUIChatPluginSpreadsheet, then fix what was found (excluding the scoria backlog this time).

🤖 Generated with Claude Code

work in GUIChatPluginSpreadsheet

isLegalMove read board[row][col] before checking the position, so a
negative or fractional row threw instead of returning an invalid move.
Adds isOnBoard, a test script on the plugins' tsx --test setup, and the
test step in CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedtsx@​4.23.151001008191100

View full report

@isamu

isamu commented Oct 21, 2026

Copy link
Copy Markdown
Member Author

CODEX VERDICT: LGTM\n\nRead SHA: f1c5263\n\n- correctness and edge cases: none\n- security (injection / path traversal / code evaluation): none\n- tests: none\n- API / schema / wire / on-disk compatibility: none\n- consistency with the rest of the codebase: none\n- input validation (negative, fractional, NaN, Infinity, non-number row/col at every entry point): none\n- CI (new test step runs on every matrix cell and cannot pass vacuously): none\n\nChecks run: �[2K�[1G�[2m$ tsx --test tests/*.test.ts�[22m
(node:72155) Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set.
(Use node --trace-warnings ... to show where the warning was created)
▶ isOnBoard
�[32m✔ accepts every square, corners included �[90m(0.419208ms)�[39m�[39m
�[32m✔ rejects positions just outside each edge �[90m(0.085083ms)�[39m�[39m
�[32m✔ rejects values that are not whole numbers �[90m(0.08625ms)�[39m�[39m
�[32m✔ rejects a non-number from an untyped caller �[90m(0.747709ms)�[39m�[39m
�[32m✔ isOnBoard �[90m(2.050166ms)�[39m�[39m
▶ playGo move coordinates
�[32m✔ a square on the board is played �[90m(0.460042ms)�[39m�[39m
�[32m✔ a square off the board or between squares is an invalid move, not a throw �[90m(0.768458ms)�[39m�[39m
�[32m✔ playGo move coordinates �[90m(1.335375ms)�[39m�[39m
�[34mℹ tests 6�[39m
�[34mℹ suites 2�[39m
�[34mℹ pass 6�[39m
�[34mℹ fail 0�[39m
�[34mℹ cancelled 0�[39m
�[34mℹ skipped 0�[39m
�[34mℹ todo 0�[39m
�[34mℹ duration_ms 152.298625�[39m, �[2K�[1G�[2m$ vue-tsc --noEmit�[22m, �[2K�[1G�[2m$ eslint src demo�[22m
�[0m
�[4m/private/tmp/claude-501/-Users-isamu-ss-llm-GUIChatPlugins-GUIChatPluginSpreadsheet/33f90efa-9e7e-4395-9044-51e33d96a56b/scratchpad/plug/GUIChatPluginGo/demo/App.vue�[24m
�[2m47:23�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m47:23�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m64:16�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m64:16�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m65:38�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m65:44�[22m �[33mwarning�[39m Unexpected any. Specify a different type �[2m@typescript-eslint/no-explicit-any�[22m
�[2m79:23�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m

�[4m/private/tmp/claude-501/-Users-isamu-ss-llm-GUIChatPlugins-GUIChatPluginSpreadsheet/33f90efa-9e7e-4395-9044-51e33d96a56b/scratchpad/plug/GUIChatPluginGo/src/core/logic.ts�[24m
�[2m20:16�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m46:17�[22m �[33mwarning�[39m Forbidden non-null assertion �[2m@typescript-eslint/no-non-null-assertion�[22m
�[2m178:23�[22m �[33mwarning�[39m Forbidden non-null assertion �[2m@typescript-eslint/no-non-null-assertion�[22m

�[4m/private/tmp/claude-501/-Users-isamu-ss-llm-GUIChatPlugins-GUIChatPluginSpreadsheet/33f90efa-9e7e-4395-9044-51e33d96a56b/scratchpad/plug/GUIChatPluginGo/src/core/plugin.ts�[24m
�[2m39:22�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m
�[2m50:22�[22m �[33mwarning�[39m Do not use any type assertions �[2m@typescript-eslint/consistent-type-assertions�[22m

�[4m/private/tmp/claude-501/-Users-isamu-ss-llm-GUIChatPlugins-GUIChatPluginSpreadsheet/33f90efa-9e7e-4395-9044-51e33d96a56b/scratchpad/plug/GUIChatPluginGo/src/core/types.ts�[24m
�[2m24:13�[22m �[33mwarning�[39m Use an interface instead of a type �[2m@typescript-eslint/consistent-type-definitions�[22m

�[33m�[1m✖ 13 problems (0 errors, 13 warnings)�[22m�[39m
�[33m�[1m 0 errors and 1 warning potentially fixable with the --fix option.�[22m�[39m
�[0m, and �[2K�[1G�[2m$ vite build && vue-tsc -p tsconfig.build.json --emitDeclarationOnly�[22m
vite v8.3.4 building client environment for production...
transforming...
�[32m✓�[0m 14 modules transformed.
rendering chunks...
computing gzip size...
�[2mdist/�[0m�[35mstyle.css �[0m�[1;2m11.96 kB�[0m�[2m │ gzip: 3.27 kB�[0m
�[2mdist/�[0m�[36mcore.js �[0m�[1;2m 0.15 kB�[0m�[2m │ gzip: 0.14 kB�[0m
�[2mdist/�[0m�[36mindex.js �[0m�[1;2m 0.15 kB�[0m�[2m │ gzip: 0.14 kB�[0m
�[2mdist/�[0m�[36mvue.js �[0m�[1;2m 7.70 kB�[0m�[2m │ gzip: 2.68 kB�[0m
�[2mdist/�[0m�[36msamples-DjlvmXdj.js �[0m�[1;2m10.88 kB�[0m�[2m │ gzip: 3.50 kB�[0m

transforming...
�[32m✓�[0m 14 modules transformed.
rendering chunks...
computing gzip size...
�[2mdist/�[0m�[35mstyle.css �[0m�[1;2m11.96 kB�[0m�[2m │ gzip: 3.27 kB�[0m
�[2mdist/�[0m�[36mcore.cjs �[0m�[1;2m 0.26 kB�[0m�[2m │ gzip: 0.21 kB�[0m
�[2mdist/�[0m�[36mindex.cjs �[0m�[1;2m 0.26 kB�[0m�[2m │ gzip: 0.21 kB�[0m
�[2mdist/�[0m�[36mvue.cjs �[0m�[1;2m 8.09 kB�[0m�[2m │ gzip: 2.41 kB�[0m
�[2mdist/�[0m�[36msamples-FcQr5l1D.cjs �[0m�[1;2m 8.93 kB�[0m�[2m │ gzip: 3.05 kB�[0m

✓ built in 263ms passed. Lint emitted only existing warnings.\n\nFINDINGS COMPLETE: I read every hunk of the diff and this is every finding I have.

@isamu
isamu merged commit 711c06f into main Oct 21, 2026
15 checks passed
@isamu
isamu deleted the fix/35-move-position-bounds branch October 21, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

playGo throws on a negative or fractional row/col instead of rejecting the move

1 participant