Skip to content

Strip Dockhand control/selector env vars from compose injection - #85

Open
wintech147 wants to merge 1 commit into
Finsys:mainfrom
wintech147:feature/1PasswordAndSecretIntegration
Open

Strip Dockhand control/selector env vars from compose injection#85
wintech147 wants to merge 1 commit into
Finsys:mainfrom
wintech147:feature/1PasswordAndSecretIntegration

Conversation

@wintech147

Copy link
Copy Markdown

Summary

Hardens the agent's handling of resolved secrets for hawser-edge deployments and
adds the first unit tests for the compose env-injection path.

Dockhand resolves secrets server-side and sends them merged into the deploy
request's envVars map. The agent injects those into the docker compose child
process environment. This change ensures Dockhand's bulk-pull control variables
(DOCKHAND_SECRET_SELECTOR, OP_ENVIRONMENT_ID) are never injected into the compose
process or a container, even when the server leaves them in the request.

Motivation

Dockhand strips the selector variable from outgoing vars only on the successful
bulk-resolution path (resolveProviderEnvVars in src/lib/server/stacks.ts). In
three misconfiguration cases (no provider bound, unregistered provider type, or a
provider without bulk support), the server logs a warning and leaves the selector
variable in envVars, which is then dispatched to the agent. Without agent-side
handling, that selector would be injected into the deployed container.

The selector names match Dockhand's BULK_SELECTOR_VARS
(['DOCKHAND_SECRET_SELECTOR', 'OP_ENVIRONMENT_ID']).

Changes

  • internal/docker/compose.go
    • Add controlEnvKeys denylist for Dockhand control/selector variables.
    • Extract the env-injection loop from Execute into a testable buildComposeEnv
      helper that validates key names, strips control/selector variables, drops
      dangerous keys, and returns the entries to append to cmd.Env.
    • Execute now calls the helper. Behavior for valid, invalid, and dangerous keys
      is unchanged.
  • internal/docker/compose_test.go (new)
    • Unit tests: secret injection, selector stripping (including case-insensitive),
      dangerous-key blocking, invalid-key rejection, and preservation of values that
      contain =.
  • Dockerfile.dev
    • Use ln -sf for the compose cli-plugin symlink. The current Wolfi
      docker-compose package already ships the symlink, so ln -s fails the build
      with "File exists". This matches the existing fix in the production Dockerfile.

Behavior

  • Resolved secrets continue to be injected into the compose process environment
    only. They are not written to disk and their values are not logged.
  • DOCKHAND_SECRET_SELECTOR and OP_ENVIRONMENT_ID are stripped before injection,
    unconditionally and case-insensitively, at debug log level.
  • Existing key validation and the deniedEnvKeys denylist are preserved.

Testing

  • go vet ./..., go build ./..., and go test ./... all pass (run in a
    golang:1.26 container).
  • New TestBuildComposeEnv_* cases cover the injection and stripping logic.
  • Built a linux/arm64 image from Dockerfile.dev and confirmed the binary runs and
    docker compose resolves inside the image.
  • End-to-end leak check with a canary secret on a hawser-edge deployment: the
    container receives the resolved value, no selector variables reach the container,
    and the canary appears in no .env, no stack file, and no agent log.

Risk and compatibility

  • No wire-format or API change. ComposeOperation is unchanged.
  • No change to how legitimate application env vars or secrets are injected.
  • The only new behavior is dropping two Dockhand-internal control variables that are
    not intended to reach containers. If a stack legitimately needed a variable named
    DOCKHAND_SECRET_SELECTOR or OP_ENVIRONMENT_ID in its container (not a supported
    use), it would no longer receive it.

Checklist

  • go vet, go build, go test pass
  • New unit tests added
  • No changes to the request/response contract
  • Docs/comments updated in-code

Dockhand resolves secrets server-side and merges them into the deploy
request's envVars map, which the agent injects into the docker compose
child process. Dockhand strips its bulk-pull control variables only on the
successful resolution path; in provider-misconfiguration cases the selector
is left in envVars and dispatched to the agent, where it would be injected
into the container.

Add controlEnvKeys (DOCKHAND_SECRET_SELECTOR, OP_ENVIRONMENT_ID) and strip
them before injection so they never reach the compose process or a
container. Extract the injection loop into a testable buildComposeEnv helper
and add the first unit tests for the package. Behavior for valid, invalid,
and dangerous keys is unchanged.

Also fix Dockerfile.dev to use ln -sf for the compose cli-plugin symlink,
matching the production Dockerfile; the current Wolfi docker-compose package
already ships the symlink and ln -s fails the build.
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.

1 participant