Skip to content

Add typed Docker command builders and a controlled executor seam - #45

Open
holladortun wants to merge 4 commits into
devfrom
feat/controlled-docker-executor
Open

Add typed Docker command builders and a controlled executor seam#45
holladortun wants to merge 4 commits into
devfrom
feat/controlled-docker-executor

Conversation

@holladortun

Copy link
Copy Markdown
Collaborator

Part of #11 (controlled Docker executor). This is the security-critical, fully CI-verifiable core; the real SSH transport + environment→server link are an immediate follow-up.

Summary

  • What: Replace placeholder, injection-prone deployment command construction with a typed, injection-safe command model and a pluggable executor that captures output and status.
  • Why: Today services/deployments.rs built commands by interpolating user values into shell strings (format!("docker pull {image}"), cd {service_dir} && …) and stored the join as output — a shell-injection surface (name = "x; rm -rf /"), with nothing executed and status hardcoded.

Scope

Included

  • RemoteCommand typed builder (Literal/Arg tokens) with POSIX single-quote rendering; image/compose/dockerfile builders emit Vec<RemoteCommand>.
  • validate_service_name / validate_image_reference (defense-in-depth, clear errors).
  • DeployExecutor trait + CommandOutcome/DeployRun capture types + secret-scrubbing output; PreviewExecutor default.
  • DeploymentService runs commands through an injected executor (mockable) and records the captured run + status.
  • Builder injection tests, executor capture/redaction tests, and a service-level failure-capture test.

Intentionally not included (immediate follow-up PR)

  • Real russh-backed SSH transport that executes for real.
  • server_id on environments (the deploy-tier-aligned target) via migration, and env→server→SshTarget resolution in trigger_deploy.

These satisfy #11's "typed builders", "injection impossible", "no arbitrary endpoint", and "failure output captured" criteria now; "executes over SSH" lands with the transport.

Changes

  • backend/src/libs/docker.rsRemoteCommand/Token, shell_quote (POSIX single-quote: ''\''), validate_*, builders return Vec<RemoteCommand>. User input enters only via arg(...) and is always quoted on render.
  • backend/src/libs/deploy_executor.rs (new)DeployExecutor trait (async-trait, already a dep), CommandOutcome/DeployRun, to_output() + redact_secrets(), PreviewExecutor.
  • backend/src/services/deployments.rsArc<dyn DeployExecutor> (preview default, with_executor for tests); trigger_deploy/restart_service build typed commands, run the executor, capture output + status (deploy_status maps preview→queued, real→succeeded/failed).
  • backend/src/libs/mod.rs — register module.
  • backend/tests/deployments_executor_tests.rs (new) — failing executor → status=failed with captured stderr; executor only receives quoted commands.

Tests

  • make backend-fmt — clean
  • make backend-test — full suite green under CI env (WARA_TEST_DATABASE_URL + WARA_DB_PUSH_SCHEMA=true): lib 24/24 (incl. injection + executor), api 10, auth 7, deployments_executor 2, migrations/persistence all pass
  • Other — cargo clippy --workspace --all-targets -- -D warnings → 0 warnings
  • make frontend-build — not applicable

Injection proof: each malicious payload (web; rm -rf /, $(touch pwned), backticks, pipes, newlines, embedded quotes) is asserted to round-trip through quote/unquote, i.e. a shell parses the rendered arg as exactly the payload — no metacharacter escapes its word.

Security Notes

  • Secret handling: No secrets flow into these commands yet; DeployRun::to_output already scrubs a provided secret list (tested with sentinels) so captured output can't leak secrets when credential/env binding lands.
  • Auth/RBAC impact: None. No new routes; the public deploy API is unchanged (the default executor keeps behavior stable). No arbitrary-command, raw-SSH, or Docker-socket endpoint is added.
  • Telemetry/logging impact: None.

Notes

Deliberate deviation: introduces a trait + Arc<dyn> seam (the codebase otherwise avoids traits) — mandated by #11's "mocked SSH executor tests" requirement; it isolates the future SSH transport behind a testable boundary.

Replace placeholder, injection-prone deployment command construction with a
typed, injection-safe model and a pluggable executor that captures output.

- libs/docker.rs: introduce RemoteCommand (Literal/Arg tokens). User-controlled
  values enter only as Arg and are POSIX single-quoted on render, so a service
  name, image reference, or path can never break out of its shell word. The
  image/compose/dockerfile builders now emit Vec<RemoteCommand> instead of
  interpolated strings. Add validate_service_name / validate_image_reference for
  defense-in-depth and clear rejection of unsafe input.
- libs/deploy_executor.rs: add the DeployExecutor trait plus CommandOutcome /
  DeployRun capture types and secret-scrubbing output. PreviewExecutor renders
  commands without connecting, so deployment output is honest and injection-safe
  until the real SSH transport lands.
- services/deployments.rs: DeploymentService runs commands through an injected
  executor (preview by default, mockable in tests) and records the captured run
  and status. No arbitrary-command endpoint is added.
- Tests: builder injection tests prove malicious names/paths render as a single
  quoted word (round-trip), executor tests prove success/failure capture and
  redaction, and a service-level test proves a failing run surfaces as a failed
  deployment with the captured output.

Real russh-backed SSH execution and the environment-to-server link are the
immediate follow-up.

@lordsarcastic lordsarcastic 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.

Tiny change

Comment thread backend/src/libs/deploy_executor.rs Outdated
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.

2 participants