Skip to content

fix: Sanitize --jitconfig value before using it - #276

Merged
konradasb merged 1 commit into
mainfrom
sanitize-jit-config
Nov 7, 2025
Merged

konradasb merged 1 commit into
mainfrom
sanitize-jit-config

Conversation

@konradasb

@konradasb konradasb commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

Description

Prevent possible command injection vulnerability via received --jitconfig token

Related Issue(s)

Fixes #274

Checklist

  • I have read the contribution guidelines.
  • I have signed off my commits.
  • I have added necessary documentation (if appropriate).
  • I have added tests to cover my changes.

Summary by CodeRabbit

  • Bug Fixes
    • Configuration input is now validated to reject invalid characters and trim unnecessary whitespace.

@coderabbitai

coderabbitai Bot commented Nov 7, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The change adds input sanitization to the Run method in runner/runner.go to address a command injection vulnerability. The modification trims whitespace from r.config and validates it against disallowed characters, returning an error if invalid characters are detected before passing it to command execution.

Changes

Cohort / File(s) Summary
Command Injection Protection
runner/runner.go
Adds strings import; implements input validation by trimming whitespace and checking r.config for disallowed shell metacharacters; returns error "invalid characters in config" on validation failure; uses sanitized config in exec.CommandContext call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Character validation set: Verify the complete list of disallowed characters aligns with shell injection vectors and doesn't exclude legitimate configuration values
  • Whitespace trimming: Confirm that trimming doesn't inadvertently break valid configurations that require leading/trailing spaces
  • Error message consistency: Ensure the error message and handling pattern match existing codebase conventions
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sanitize-jit-config

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 330b0df and de1cad9.

📒 Files selected for processing (1)
  • runner/runner.go (2 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@konradasb
konradasb merged commit 2392a51 into main Nov 7, 2025
3 of 4 checks passed
@konradasb
konradasb deleted the sanitize-jit-config branch November 7, 2025 07:19
@hostingerbot hostingerbot Bot mentioned this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security Analysis Report - Potential Command Injection Vulnerability in runner.go

1 participant