Repository navigation
fix(tools): close ROADMAP-02/03/04 — markdown escaping, bounded reads, finite results - #120
Merged
Merged
Conversation
…a prior stat The cap was checked via stat().st_size and then enforced by a second open, so a file grown between the two calls was read in full. One bounded read of max_bytes + 1 closes the check-then-read window; an exact-cap file stays fully readable. Regression test simulates the race with a lying st_size. Closes #95 (ROADMAP-03)
…air guard Float overflow (1e308 * 10 ** 3) previously rendered 'inf' into the prompt, and an overflowing float literal (1e999) silently parsed to inf at AST level and flowed through every operation. The result is now checked with math.isfinite at the exit, and non-finite literals are rejected at the operand boundary. The bare 1000/100 literals in the Pow guard become MAX_POW_BASE_MAGNITUDE / MAX_POW_PAIR_EXPONENT with unchanged values. Closes #96 (ROADMAP-04)
Cell values were inserted into the markdown table unescaped, so a value containing a pipe, newline or control character could forge table rows and smuggle prompt structure — stored prompt injection through database data. Escapes are applied per cell (backslash first), the rendered cell length is capped (MAX_CELL_CHARS), and column names get the same treatment. Closes #94 (ROADMAP-02)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Small stacked hardening batch closing three P0
ROADMAPissues from the 2026-10-27 assessment. All changes are confined toagentkit/tools/builtin/plus their unit tests, so no other in-flight work conflicts with this PR.Closes #94 (ROADMAP-02 — SQL markdown cell escaping)
Closes #95 (ROADMAP-03 — read_file check-then-read race)
Closes #96 (ROADMAP-04 — calculator non-finite results)
What changed
#94
sql_readonly— escape and bound rendered cells_format_cell/_escape_cell_textrewrite each cell: backslash first, then|→\|, newline/CR/tab →\n/\r/\tescapes, and every other C0/DEL control char →\uXXXX.MAX_CELL_CHARS(200) with a[Truncated]marker, so one wide column can't eat the executor budget; column names get the same treatment.#95
read_file— cap enforced on the bytes actually readstat().st_sizecheck followed by a second open; a file grown between the two was read in full. One boundedread(max_bytes + 1)replaces it; oversize still fails with the same user-visible phrase, and an exact-cap file stays fully readable.#96
calculator— reject non-finite results1e308 * 10 ** 3previously renderedinfinto the prompt. The final result is checked withmath.isfinite; the propagatinginf/nanbecomes a structuredValueError.1e999→ inf at parse time) are rejected at the operand boundary.1000/100literals in thePowguard becomeMAX_POW_BASE_MAGNITUDE/MAX_POW_PAIR_EXPONENTwith unchanged values (behaviour preserved; existing guard tests pass untouched).Tests
test_tool_calculator.py,test_tool_read_file.py,test_tool_sql_readonly.py(incl. one lying-st_sizeregression that fails on the old read code, and adversarial-cell scenarios both unit and end-to-end through the sqlite fixture).pytest: 494 passed, 30 skipped (was 483 passed); real-infra tier unchanged.ruff check .,ruff format --check .,mypy agentkit tests: clean.mypy agentkit tests exampleshits the pre-existingdemo.pydual-module-name error onmain(CI correctly runsmypy agentkit tests).