Skip to content

feat: add diagnostic command injection challenge to mechanic service - #371

Open
gabrielfrdev wants to merge 3 commits into
OWASP:developfrom
gabrielfrdev:feat/direct-command-injection-challenge
Open

gabrielfrdev wants to merge 3 commits into
OWASP:developfrom
gabrielfrdev:feat/direct-command-injection-challenge

Conversation

@gabrielfrdev

Copy link
Copy Markdown

Description

This PR introduces a new Direct Command Injection challenge to the workshop mechanic service.

The goal is to demonstrate how unsafe handling of shell commands—specifically using subprocess.run with shell=True on user-controlled input—leads to Remote Code Execution (RCE). Users can now exploit the diagnostic_command parameter in the ReceiveReportView to execute arbitrary commands on the host.

Testing

  • Unit Tests: Added new test cases in services/workshop/crapi/mechanic/tests.py and services/workshop/crapi/merchant/tests.py to verify:
    1. Normal diagnostic output forwarding.
    2. Successful command execution via injection payloads (e.g., using command separators like ;).
  • Manual Verification: Verified locally by sending a request with diagnostic_command=status; ls -la and confirming the host's directory listing in the response.

Documentation

  • Updated openapi-spec/crapi-openapi-spec.json to include the new parameter and response fields.
  • Updated docs/challenges.md and docs/challengeSolutions.md to include instructions and the solution for this new challenge.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my feature works
  • Any dependent changes have been merged
  • I have documented any changes if required in the docs.

"sent": True,
"report_link": report_link,
}
diagnostic_command = report_details.get("diagnostic_command")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

RCE should be only enabled when the ENABLE_SHELL_INJECTION env is injected. So its safe when users deploy with public access which should be default behavior

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the review. I pushed an update that gates the diagnostic shell execution behind ENABLE_SHELL_INJECTION, with the default set to false for public deployments.

I also wired the flag into the Workshop deployment config, added coverage for the default-disabled and enabled challenge paths, and updated the docs/OpenAPI to make the lab-only behavior explicit.

Validated locally with the Workshop test suite and the full Postman/Newman collection.

@piyushroshan
piyushroshan requested a balanced review from Copilot November 6, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Non-UTF-8 command output can trigger an uncaught UnicodeDecodeError, causing HTTP 500 after the service request is created instead of returning diagnostic output.

1 open issue
What changed in this PR

This PR adds an opt-in command injection challenge to the Workshop mechanic report flow, allowing learners to execute diagnostic commands and inspect their output.

Changes:

  • Adds the diagnostic parameter, shell execution, and response output behind ENABLE_SHELL_INJECTION.
  • Adds tests for disabled behavior, normal output, command injection, and merchant forwarding.
  • Documents Challenge 19, updates both API specifications, and adds deployment configuration.
File Description
services/​workshop/​crapi/​merchant/​tests.py Tests diagnostic parameter forwarding and returned output.
services/​workshop/​crapi/​merchant/​serializers.py Accepts the optional diagnostic command.
services/​workshop/​crapi/​mechanic/​views.py Adds gated shell execution and diagnostic output handling.
services/​workshop/​crapi/​mechanic/​tests.py Tests disabled, normal, and injected diagnostic requests.
services/​workshop/​crapi/​mechanic/​serializers.py Accepts diagnostic commands in report requests.
services/​chatbot/​src/​resources/​crapi-openapi-spec.json Documents diagnostic fields in the chatbot API specification.
openapi-spec/​crapi-openapi-spec.json Documents diagnostic parameters and response output.
docs/​challengeSolutions.md Adds the Challenge 19 walkthrough.
docs/​challenges.md Describes the new challenge and enablement flag.
deploy/​k8s/​base/​workshop/​config.yaml Defaults shell injection to disabled.
deploy/​helm/​templates/​workshop/​config.yaml Exposes the shell injection flag through Helm values.
deploy/​docker/​docker-compose.yml Passes the opt-in flag to Workshop, defaulting to disabled.

Review effort: Balanced

stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
shell=True,
text=True,

This branch has not been deployed

No deployments
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.

3 participants