Security and robustness hardening - #30
Merged
Merged
Conversation
Core hardening: - Symlink/TOCTOU-safe file writes (open_secure_write), owner-only perms - Versioned, fingerprinted checkpoints; fail-safe against new config fields - Trusted-remote verification for self-update; explicit refs/heads/main pull - Rich-markup escaping of attacker-influenced output; CSV formula-injection guard - CRLF/NUL header validation; structured FUZZ/param substitution (no regex) - TOML config type validation and unknown-key warnings Review fixes: - Fix pre-commit hooks: drop unsupported `run --staged`; correct pip-audit --desc - Dedup composite-OS CSV rows (no duplicate requests; 958 default cases) - Partial transient request failures no longer fail an otherwise good scan - Text output files no longer hard-wrap long paths - Non-interactive EOF no longer auto-approves OS narrowing - CLI headers extend config headers with CLI precedence - Consistent header deprecation warnings and FUZZ detection Detection change (intentional): full response comparison replaces Content-Length-only shortcut; quick_ratio() upper-bound preserves perf. Pending end-to-end validation against a live LFI target. CI: SHA-pinned GitHub Actions, least-privilege permissions, Dependabot, minimum-version job. 268 tests pass.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughPanoptic adds stricter configuration and validation, OS-aware case expansion, secure output and checkpoint handling, revised scanning and retry behavior, hardened update logic, expanded output redaction, and CI/pre-commit automation with corresponding tests and documentation. ChangesPanoptic hardening and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Scanner
participant NetworkClient
participant Output
CLI->>Config: merge arguments and config
Config->>Scanner: create ScanConfig
Scanner->>NetworkClient: fetch scan responses
NetworkClient-->>Scanner: responses or failures
Scanner->>Output: write findings and status
Output-->>CLI: return exit code
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.githooks/pre-commit:
- Around line 99-103: Update the npx fallback in the pre-commit hook to invoke
the CLI package via markdownlint-cli instead of the library package
markdownlint, while preserving the existing staged-file arguments and
CHECKS_RAN/EXIT_CODE handling.
In @.github/workflows/ci.yml:
- Line 20: Disable persisted checkout credentials by adding persist-credentials:
false to each actions/checkout step in .github/workflows/ci.yml at lines 20, 33,
and 50; no other workflow behavior needs to change.
In `@panoptic/config.py`:
- Around line 131-136: Update the legacy header migration in the configuration
merge logic to reject unsupported types instead of silently dropping them. Keep
string and list values mapped to merged["headers"], but raise the same
established configuration/type error used for other malformed TOML values when
legacy_headers is neither a str nor a list.
- Around line 177-182: Update the output_format handling in the configuration
merge/validation flow so every present value, including non-strings, is
validated and converted to OutputFormat rather than bypassing validation. Ensure
invalid types and values raise TypeError or ValueError for the existing run()
error handler, while preserving valid enum instances and string conversion
behavior.
In `@tests/test_core.py`:
- Around line 624-625: Bound the wait loop in the test around save_started so it
cannot spin indefinitely when _flush_checkpoint exits without starting a save.
Add a deadline or apply asyncio.wait_for to the wait operation, and ensure
timeout causes the test to fail rather than hang.
- Line 246: Guard the 0o600 permission assertion in test_no_filename_collision
with the same os.name != "posix" skipif used by
test_discovered_file_write_refuses_symlink and TestScanOutputFilePermissions,
while leaving the collision behavior assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a0e8c85-b947-4a60-b553-da53a61566f3
⛔ Files ignored due to path filters (1)
panoptic/data/cases.csvis excluded by!**/*.csv
📒 Files selected for processing (33)
.gitattributes.githooks/pre-commit.github/constraints-min.txt.github/dependabot.yml.github/workflows/ci.yml.gitignore.pre-commit-config.yamlREADME.mdpanoptic/__main__.pypanoptic/cases.pypanoptic/cli.pypanoptic/config.pypanoptic/core.pypanoptic/heuristic.pypanoptic/models.pypanoptic/network.pypanoptic/output.pypanoptic/parsers.pypanoptic/update.pypanoptic/utils.pypyproject.tomltests/test_cases.pytests/test_cli.pytests/test_config.pytests/test_core.pytests/test_heuristic.pytests/test_integration.pytests/test_models.pytests/test_network.pytests/test_output.pytests/test_parsers.pytests/test_update.pytests/test_utils.py
👮 Files not reviewed due to content moderation or server errors (14)
- panoptic/core.py
- tests/test_cli.py
- panoptic/heuristic.py
- panoptic/parsers.py
- panoptic/update.py
- tests/test_heuristic.py
- tests/test_parsers.py
- tests/test_models.py
- tests/test_update.py
- panoptic/network.py
- tests/test_integration.py
- tests/test_network.py
- panoptic/output.py
- tests/test_output.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
panoptic/config.py (1)
146-164: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDo not skip validation for falsey configuration values.
The truthiness checks silently discard malformed values such as
proxy.url = 0,headers.cookie = false, orheaders.values = "", so_validate_merged_types()never rejects them. Useis not Nonefor these optional fields and add regression cases totests/test_config.py.🛠️ Proposed fix
- if proxy_url: + if proxy_url is not None: merged["proxy"] = proxy_url ... - if header_ua: + if header_ua is not None: merged["user_agent"] = header_ua ... - if header_cookie: + if header_cookie is not None: merged["cookie"] = header_cookie ... - if header_values: + if header_values is not None: if not isinstance(header_values, list) or not all( isinstance(header, str) for header in header_values ):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@panoptic/config.py` around lines 146 - 164, Update the optional-field checks in the configuration merge logic around proxy_url, header_ua, header_cookie, and header_values to use explicit None checks, ensuring falsey values are copied into merged and reach _validate_merged_types() for rejection. Add regression cases in tests/test_config.py covering malformed falsey values such as proxy.url=0, headers.cookie=false, and headers.values="".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.githooks/pre-commit:
- Line 100: Update the markdownlint invocation in the pre-commit hook to prevent
npx from auto-installing packages: use a declared, pinned/preinstalled
markdownlint-cli binary or invoke npx with --no-install. Ensure the hook fails
clearly when markdownlint-cli is unavailable.
---
Outside diff comments:
In `@panoptic/config.py`:
- Around line 146-164: Update the optional-field checks in the configuration
merge logic around proxy_url, header_ua, header_cookie, and header_values to use
explicit None checks, ensuring falsey values are copied into merged and reach
_validate_merged_types() for rejection. Add regression cases in
tests/test_config.py covering malformed falsey values such as proxy.url=0,
headers.cookie=false, and headers.values="".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 439932e0-a01c-4a86-8f9e-a523e7e6cf72
📒 Files selected for processing (5)
.githooks/pre-commit.github/workflows/ci.ymlpanoptic/config.pytests/test_config.pytests/test_core.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.githooks/pre-commit (1)
49-84: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAlign the fallback with
.pre-commit-config.yaml.
The fallback uses different checks from the pinned hooks (ruff check/ruff format --check/pyrighton PATH vs.ruff --fix/ruff-format/mypy --strict). Ifpre-commitisn’t installed, reuse the same toolchain/options or fail fast so commits aren’t validated differently across environments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.githooks/pre-commit around lines 49 - 84, The Python fallback checks in the pre-commit hook must match the pinned hooks in .pre-commit-config.yaml. Update the Python checks block to reuse the configured ruff --fix, ruff-format, and mypy --strict toolchain/options, or fail fast when those dependencies are unavailable; remove the current ruff check, ruff format --check, and pyright path so validation is consistent across environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.githooks/pre-commit:
- Around line 49-84: The Python fallback checks in the pre-commit hook must
match the pinned hooks in .pre-commit-config.yaml. Update the Python checks
block to reuse the configured ruff --fix, ruff-format, and mypy --strict
toolchain/options, or fail fast when those dependencies are unavailable; remove
the current ruff check, ruff format --check, and pyright path so validation is
consistent across environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44e3026c-7dae-40ad-8ff3-69486e2c1e6e
📒 Files selected for processing (1)
.githooks/pre-commit
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
1 participant
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.
Summary
Verification
pip checkSummary by CodeRabbit
--version, richer--listoutput (JSON/CSV/text), and improved proxy support (SOCKS5/SOCKS5H).