Skip to content

fix: sanitize plugin command placeholders#625

Open
saurabhhhcodes wants to merge 1 commit into
utksh1:mainfrom
saurabhhhcodes:fix/sanitize-input-615
Open

fix: sanitize plugin command placeholders#625
saurabhhhcodes wants to merge 1 commit into
utksh1:mainfrom
saurabhhhcodes:fix/sanitize-input-615

Conversation

@saurabhhhcodes
Copy link
Copy Markdown
Contributor

Summary

  • call the existing sanitize_input() helper for every user-controlled plugin placeholder before command construction
  • strip leading - prefixes from sanitized values so placeholder inputs cannot become tool flags like --debug
  • add regression coverage for shell metacharacter stripping and leading-option-prefix removal during interpolation

Fixes #615

Testing

  • /Users/saurabhkumarbajpaiai/.cache/codex-runtimes/codex-primary-runtime/dependencies/python/bin/python3 -m py_compile backend/secuscan/plugins.py backend/secuscan/validation.py testing/backend/unit/test_plugins.py
  • direct targeted interpolation assertions with Python 3.12:
    • sanitize_input('--debug;$(whoami)') == 'debugwhoami'
    • _interpolate('{templates}', {'templates': '--debug;$(whoami)'}) == 'debugwhoami'
    • _interpolate('--user-agent={user_agent}', {'user_agent': '--verbose|curl'}) == '--user-agent=verbosecurl'
  • git diff --check

Note

I attempted the focused pytest command for testing/backend/unit/test_plugins.py testing/backend/unit/test_validation.py, but local dependency installation is blocked by the native pycairo/cairo requirement pulled through xhtml2pdf. The code-level targeted checks above pass under Python 3.12.

@utksh1 utksh1 added level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jun 7, 2026
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The placeholder hardening is security-relevant, but the PR is not merge-ready with backend-lint/frontend-checks failing. Please rebase on the current CI baseline and get checks green. Also add/keep coverage for legitimate placeholder values so the sanitizer does not break valid non-shell argv use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] sanitize_input() is Dead Code — No Shell Metacharacter Stripping Exists

2 participants