Skip to content

fix: sanitize plugin command placeholders - #625

Merged
utksh1 merged 5 commits into
utksh1:mainfrom
saurabhhhcodes:fix/sanitize-input-615
Jun 8, 2026
Merged

fix: sanitize plugin command placeholders#625
utksh1 merged 5 commits 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

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

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.

@utksh1 utksh1 added the gssoc:approved Admin validation: approved for GSSoC scoring label Jun 8, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed the updated placeholder sanitization diff. It is focused on plugin command interpolation, includes regression coverage for dangerous placeholder values and safe argv values, and current checks are green. Approved pending update with main and fresh CI.

@utksh1
utksh1 merged commit 2686ba2 into utksh1:main Jun 8, 2026
11 checks passed
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 gssoc:approved Admin validation: approved for GSSoC scoring 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