Keep evidence harness reports public-safe#9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the evidence-harness Python scripts to keep generated reports “public-safe” by avoiding leakage of local absolute paths and by rejecting/handling empty command lists more robustly.
Changes:
- Sanitize scenario contract report paths (and directory error strings) using a public-safe path formatter.
- Reject empty
commandarrays when loading a test matrix and failrun_entrycleanly for empty commands. - Add regression tests for empty-command handling and for path-safety in scenario contract reports.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/strix_test_matrix.py | Rejects empty command lists at load time; makes run_entry fail safely for empty commands. |
| scripts/strix_scenario_contract.py | Introduces public_path() and uses it in scenario/directory reports and directory error strings. |
| python/tests/test_strix_test_matrix.py | Adds regression tests for rejecting empty matrix commands and for run_entry behavior. |
| python/tests/test_strix_scenario_contract.py | Adds assertions that reports/error strings do not contain the local temp directory path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| "path": str(path.relative_to(ROOT) if path.is_relative_to(ROOT) else path), | ||
| "path": public_path(path), | ||
| "scenario_id": scenario_id, |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b962b10d22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status = "failed" if errors else "passed" | ||
| return { | ||
| "path": str(path.relative_to(ROOT) if path.is_relative_to(ROOT) else path), | ||
| "path": public_path(path), |
There was a problem hiding this comment.
The new public-safe path handling is only applied in the normal return path, so malformed/unreadable scenario files can still leak absolute local paths via the early exception return in validate_scenario ("path": str(path)) and its raw exception text. In practice, running the contract against external directories with a bad YAML or read error will still publish host-specific filesystem details, which contradicts the public-safe reporting goal of this change. Please ensure the exception branch also uses public_path(path) (and avoid path-bearing raw exception strings).
Useful? React with 👍 / 👎.
Follow-up to Copilot review on PR #8.\n\nIncluded:\n- use public-safe path labels in scenario contract reports and error strings\n- avoid leaking external absolute paths in generated evidence reports\n- reject empty matrix command lists during matrix loading\n- make run_entry return a failed result for empty command lists instead of risking a subprocess crash\n- add focused regression tests for both cases\n\nValidation:\n- python scripts/verify_public_surface.py -> passed\n- pytest -q -> 138 passed\n- pytest -q python/tests/test_strix_test_matrix.py python/tests/test_strix_scenario_contract.py -> 15 passed\n- python scripts/strix_scenario_contract.py --scenario-dir /tmp/does-not-exist --output /tmp/strix-missing-scenario-dir-public-safe.json -> exits nonzero with public-safe failure report\n- python scripts/strix_test_matrix.py --select smoke --output /tmp/strix-smoke-public-safe.json -> passed\n- cargo test -p strix-optimizer doctrine_profile_accepts_neutral_aliases --lib -> passed\n- cargo check -p strix-python -> passed