Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change adds a CodeQL library pack and Python models for log-injection and path-injection barriers, including a guard for ChangesCodeQL Python Models
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoModel PRKS sanitizer boundaries in CodeQL
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
 |
Code Review by Qodo
1. Unsafe fallbacks evade log scanning
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| # arbitrary request/library text. Keep these barriers query-specific so | ||
| # they do not suppress unrelated taint flows. | ||
| - ["backend.log_safety", "Member[safe_log_id].ReturnValue", "log-injection"] | ||
| - ["backend.log_safety", "Member[safe_log_label].ReturnValue", "log-injection"] |
There was a problem hiding this comment.
1. Unsafe fallbacks evade log scanning 🐞 Bug ⚙ Maintainability
The safe_log_label return-value barrier treats every result as sanitized, but the function returns its caller-supplied fallback unchanged on rejected values. When untrusted text is supplied as the fallback and the primary value is empty or invalid, that text can reach a log while CodeQL terminates the flow at this helper.
Agent Prompt
## Issue description
The new CodeQL barrier declares every `safe_log_label` return value safe, although invalid primary values cause the function to return its `fallback` argument unchanged.
## Fix Focus Areas
- .github/codeql/extensions/prks-python-models/models/prks.yml[9-10]
- backend/log_safety.py[194-202]
## Recommended Fix
Make `safe_log_label` validate and constrain the fallback with the same log-safe policy, using a fixed safe value such as `unknown` if the fallback itself is invalid. Keep the return-value barrier only after every return path is guaranteed to produce constrained text, and add a test using a fallback containing a newline.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
Summary
Teach CodeQL about PRKS's existing Python sanitization/containment boundaries so the large
py/log-injectionandpy/path-injectionfamilies can be evaluated with the same contracts the runtime already enforces.This is deliberately query-specific modeling, not a global suppression of either rule.
Why
The fresh
masteranalysis reported:py/log-injectionresultspy/path-injectionresultsManual inspection showed that many of these flows cross helpers that constrain output before it reaches a log or filesystem sink, but CodeQL does not infer those project-specific contracts automatically.
Changes
Add a repository-local Python CodeQL model pack under:
.github/codeql/extensions/prks-python-models/It models:
Log-injection barriers
safe_log_idsafe_log_labelsafe_error_typesafe_bind_scopesafe_client_error_kindsafe_error_namesafe_client_sourcesafe_routeThese helpers return fixed/constrained labels or normalized routes rather than raw user/library text.
Path-injection barriers
safe_pdf_path_under_dirmanaged_pdf_filenamereferenced_managed_pdf_filenamesafe_processing_path_under_dirThese helpers either return a canonical path proven to remain below the managed root or a validated single managed filename.
Path containment guard
backup_restore._path_is_under(child, parent)The guard resolves both paths via
realpathand only accepts a child equal to or beneath the requested parent.Safety of the approach
The models are limited to the relevant CodeQL query kinds:
log-injectionpath-injectionThey do not mark these values universally safe and do not disable either query. Any taint flow that does not cross one of these reviewed boundaries remains visible.
Validation
The PR CodeQL run is the acceptance test for the model pack. The expected outcome is a material reduction in the current 40 log-injection / 48 path-injection findings without changing application runtime behavior.
Any findings that remain after this PR should be reviewed individually rather than expanding the model indiscriminately.
Prepared using ChatGPT.
Summary by CodeRabbit