Skip to content

Potential fix for code scanning alert no. 9: Uncontrolled data used in path expression#53

Merged
justindobbs merged 1 commit intomainfrom
alert-autofix-9
Feb 28, 2026
Merged

Potential fix for code scanning alert no. 9: Uncontrolled data used in path expression#53
justindobbs merged 1 commit intomainfrom
alert-autofix-9

Conversation

@justindobbs
Copy link
Copy Markdown
Owner

Potential fix for https://github.com/justindobbs/Tracecore/security/code-scanning/9

General strategy: Treat all externally provided run references (run_id, replay, a, b, run_a, run_b) as untrusted and constrain them to safe identifiers before using them to construct filesystem paths. For identifiers that are meant to reference files in .agent_bench/runs, we can restrict them to a safe pattern (e.g., alphanumeric, dash, underscore) and possibly a maximum length. This avoids directory traversal and prevents using absolute or relative paths. Because we cannot modify load_run_artifact here, the most direct fix for the reported alert path is to harden load_run in runlog.py, since that’s the function CodeQL flags and it’s the one used in all the tainted flows.

Best concrete fix: Introduce a small helper in agent_bench/runner/runlog.py to validate run_id strings, then call it from both persist_run and load_run. The helper will enforce a whitelist of allowed characters (e.g., A-Za-z0-9_-) and a reasonable length limit. If validation fails, raise a ValueError. This preserves existing semantics for normal IDs (which presumably match this pattern) while making path traversal impossible and addressing all variants that pass user-controlled IDs into load_run. We do not need new imports beyond what is already there; we can define a simple check inline using str.isalnum() and a small allowed extra character set.

Concrete changes in agent_bench/runner/runlog.py:

  1. Add a new private function _validate_run_id(run_id: str) -> str near the top of the file (after the constants, before _ensure_root). This function:

    • Strips whitespace.
    • Ensures the result is non-empty.
    • Checks length (e.g., ≤ 128 characters).
    • Ensures every character is alphanumeric, -, or _.
    • Raises ValueError if any check fails.
    • Returns the cleaned run_id if valid.
  2. In persist_run, after fetching run_id = result.get("run_id") and checking for truthiness, pass it through _validate_run_id and use the validated value to build the path.

  3. In load_run, validate the incoming run_id via _validate_run_id before using it to construct the path.

These targeted edits keep behavior the same for legitimate IDs, avoid assumptions about other modules, and directly fix the uncontrolled path construction in the snippets referenced by CodeQL.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@justindobbs justindobbs marked this pull request as ready for review February 28, 2026 04:25
@justindobbs justindobbs merged commit 1ede239 into main Feb 28, 2026
8 checks passed
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.

1 participant