mirror of
https://github.com/dokku/dokku.git
synced 2026-08-29 10:08:53 +02:00
fix: prevent command injection via docker options eval
Values supplied through docker options, `--ttl-seconds`, and `-e` flowed into a Bash `eval` during build, deploy, and run, letting a low-privileged user execute arbitrary commands on the host as the dokku user. These arguments are now tokenized and passed through to the container verbatim, without shell expansion. A one-time migration repairs stored labels whose backticks were saved with a stray backslash so Traefik-style rules stay valid on the next deploy.
This commit is contained in:
@@ -64,6 +64,12 @@ Multiple docker options can also be specified in a single call. Each `--flag [va
|
||||
dokku docker-options:add node-js-app deploy "--ulimit nofile=12" "--shm-size 256m"
|
||||
```
|
||||
|
||||
Option values are stored and passed to the container verbatim. Quoting only controls how a value is split into words - no shell expansion is performed, so `$(...)`, backticks, `$VAR`, and globs are treated literally rather than being interpreted by the shell. This is what lets values such as a Traefik router rule be applied as-is:
|
||||
|
||||
```shell
|
||||
dokku docker-options:add node-js-app deploy '--label "traefik.http.routers.web.rule=Host(`node-js-app.example.com`) && PathPrefix(`/api`)"'
|
||||
```
|
||||
|
||||
A misplaced `--process PROC` (i.e. one specified after the app name instead of before it) is honored as a subcommand flag rather than stored as a docker option, so the example above and the equivalent process-scoped form below behave identically:
|
||||
|
||||
```shell
|
||||
|
||||
@@ -26,6 +26,7 @@
|
||||
- A second round of `:report` additions surfaces every remaining settable-but-unreported property under the same raw/global/computed convention so external tooling can verify drift via `:report --format json` without falling back to a generic bash task. The `ps` plugin gains `--ps-dockerfile-start-cmd` and `--ps-computed-dockerfile-start-cmd`, `--ps-start-cmd` and `--ps-computed-start-cmd`, and the `--ps-skip-deploy` / `--ps-global-skip-deploy` / `--ps-computed-skip-deploy` triple (default `false`). The `builder` plugin gains the `--builder-skip-cleanup` triple (default `false`). The `scheduler` plugin gains the `--scheduler-shell` triple. The `proxy` plugin gains the `--proxy-proxy-port` and `--proxy-proxy-ssl-port` triples and exposes the raw `disabled` property as `--proxy-disabled` / `--proxy-computed-disabled`, alongside the existing inverted `--proxy-enabled`. The `openresty` plugin gains `--openresty-global-log-level` and `--openresty-computed-log-level` (default `ERROR`). The `nginx` plugin gains the `--nginx-nginx-service-command` triple. The `scheduler-k3s` plugin gains `--scheduler-k3s-global-token`, but the value is masked as `*******` in default stdout output; the raw value is returned only when the report is requested via `--format json` or when this flag is queried explicitly by name. The traefik `dns-provider-<env_var>` keys now follow the same explicit-query rule - previously they were unmasked only for `--format json`, but a query like `dokku traefik:report --traefik-dns-provider-cf_api_key` now returns the actual value instead of `*******`.
|
||||
- All Go-implemented plugins (`app-json`, `apps`, `builder`, `buildpacks`, `builds`, `cron`, `docker-options`, `logs`, `network`, `ports`, `proxy`, `ps`, `registry`, `resource`, `scheduler`, `scheduler-k3s`, `storage`) now emit JSON keys from `:report --format json` without the `<plugin>-` head segment, matching the shape bash plugins have always emitted. For example, `dokku ps:report myapp --format json` now contains `stop-timeout-seconds`, `global-stop-timeout-seconds`, and `computed-stop-timeout-seconds` keys. The CLI flag names (`--ps-stop-timeout-seconds`, etc.) are unchanged, and `:set` semantics are unchanged. For backwards compatibility during the 0.38.x patch series, the old `<plugin>-<property>` JSON keys are emitted side-by-side with the new keys, so external scripts reading either shape continue to work. The legacy keys will be dropped in a future major release. External JSON consumers should migrate to the new key shape.
|
||||
- The `scheduler-k3s` plugin now manages env config and the dokku-generated image pull Secret as their own helm releases with stable names (`config-{app}` and `pull-secret-{app}`) rather than bundling them into the app helm chart with a per-deploy timestamp suffix (`env-{app}.{ts}` / `ims-{app}.{ts}`). This fixes two bugs: a helm rollback of the app chart no longer deletes Secrets that older ReplicaSets still reference, and the Deployment's `imagePullSecrets` list no longer accumulates references to nonexistent Secrets across deploys. The next deploy of an app switches the Deployment's `envFrom` and `imagePullSecrets` references to the stable names and prunes any leaked entries; existing live Deployments do not need to be patched manually. App rename now also uninstalls the old `tls-{app}`, `config-{app}`, and `pull-secret-{app}` releases under the previous app name; the new name's releases are recreated on the next deploy or certs sync.
|
||||
- **New in 0.38.25:** Values supplied through docker options, `dokku run`'s `-e`/`--env` flag, and `--ttl-seconds` are no longer evaluated by the shell when assembling a container's arguments; they are now tokenized and passed through verbatim. This closes a command-injection vector where a `$(...)` or backtick expression in one of these values executed on the host as the `dokku` user during build, deploy, or run. As a result, shell metacharacters such as `$(...)`, backticks, `$VAR`, and globs in these values are treated literally instead of being expanded, and `--ttl-seconds` must now be a plain integer. Existing Traefik docker-options labels (those whose label key begins with `traefik.`) whose backticks were stored with a stray backslash are repaired automatically the first time `dokku` runs after the upgrade, so they become valid on the next deploy.
|
||||
- The storage plugin now treats persistent volumes as named, scheduler-aware first-class resources via `storage:create`, `storage:mount`, `storage:set`, and `storage:destroy`. The legacy `storage:mount <app> <host>:<container>` colon form continues to work on docker-local apps but is deprecated; on k3s apps it is rejected. Existing colon-form mounts are migrated automatically the first time the new storage plugin runs (during the install trigger) - they appear as `legacy-<hash>` entries in `storage:list-entries`. The migration is idempotent and tied to a per-app flag file at `$DOKKU_LIB_ROOT/config/storage/.migrated/<app>`; deleting that file forces a re-scan on the next install. The `storage:ensure-directory` command keeps working but now emits a deprecation warning - prefer `storage:create <name> [<path>]` (the path defaults to the same `$DOKKU_LIB_ROOT/data/storage/<name>` location). Storage entry names must now be DNS-1123 labels of 45 characters or less so they can be used verbatim as Helm release and Kubernetes resource names; underscores and uppercase characters that the older `ensure-directory` validator accepted are rejected for new names. The migration synthesizer always uses lowercase hex hashes so existing data is never locked out.
|
||||
|
||||
### TLS handshake behavior change
|
||||
|
||||
@@ -36,8 +36,11 @@ trigger-builder-dockerfile-builder-build() {
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-build "$APP" "$BUILDER_TYPE")"
|
||||
DOCKER_ARGS+=" $DOKKU_GLOBAL_BUILD_ARGS"
|
||||
|
||||
DOCKER_ARGS=" $DOCKER_ARGS "
|
||||
eval set -- "$DOCKER_ARGS"
|
||||
declare -a DOCKER_ARGS_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
DOCKER_ARGS_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
set -- "${DOCKER_ARGS_ARRAY[@]}"
|
||||
|
||||
declare -a DOCKERFILE_ARGS
|
||||
while true; do
|
||||
|
||||
@@ -71,8 +71,10 @@ trigger-builder-herokuish-builder-build() {
|
||||
fi
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-build "$APP" "$BUILDER_TYPE")"
|
||||
|
||||
declare -a ARG_ARRAY
|
||||
eval "ARG_ARRAY=($DOCKER_ARGS)"
|
||||
declare -a ARG_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
ARG_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
|
||||
local DOKKU_CONTAINER_EXIT_CODE=0
|
||||
fn-builder-herokuish-ensure-cache "$APP"
|
||||
|
||||
@@ -37,8 +37,11 @@ trigger-builder-nixpacks-builder-build() {
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-build "$APP" "$BUILDER_TYPE")"
|
||||
DOCKER_ARGS+=" $DOKKU_GLOBAL_BUILD_ARGS"
|
||||
|
||||
DOCKER_ARGS=" $DOCKER_ARGS "
|
||||
eval set -- "$DOCKER_ARGS"
|
||||
declare -a DOCKER_ARGS_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
DOCKER_ARGS_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
set -- "${DOCKER_ARGS_ARRAY[@]}"
|
||||
|
||||
declare -a NIXPACKS_ARGS
|
||||
while true; do
|
||||
|
||||
@@ -41,8 +41,11 @@ trigger-builder-pack-builder-build() {
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-build "$APP" "$BUILDER_TYPE")"
|
||||
[[ "$DOKKU_TRACE" ]] && DOCKER_ARGS+=" --env=TRACE=true "
|
||||
|
||||
DOCKER_ARGS=" $DOCKER_ARGS "
|
||||
eval set -- "$DOCKER_ARGS"
|
||||
declare -a DOCKER_ARGS_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
DOCKER_ARGS_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
set -- "${DOCKER_ARGS_ARRAY[@]}"
|
||||
|
||||
declare -a PACK_ARGS
|
||||
while true; do
|
||||
|
||||
@@ -36,8 +36,11 @@ trigger-builder-railpack-builder-build() {
|
||||
local DOCKER_ARGS=$(: | plugn trigger docker-args-build "$APP" "$BUILDER_TYPE")
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-build "$APP" "$BUILDER_TYPE")"
|
||||
|
||||
DOCKER_ARGS=" $DOCKER_ARGS "
|
||||
eval set -- "$DOCKER_ARGS"
|
||||
declare -a DOCKER_ARGS_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
DOCKER_ARGS_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
set -- "${DOCKER_ARGS_ARRAY[@]}"
|
||||
|
||||
declare -a RAILPACK_ARGS
|
||||
while true; do
|
||||
|
||||
@@ -245,6 +245,12 @@ fn-plugn-trigger-exists() {
|
||||
return 1
|
||||
}
|
||||
|
||||
fn-docker-args-split() {
|
||||
declare desc="tokenizes a docker args string into NUL-delimited fields without shell expansion"
|
||||
declare DOCKER_ARGS="$1"
|
||||
printf "%s" "$DOCKER_ARGS" | xargs -r printf "%s\0"
|
||||
}
|
||||
|
||||
is_valid_app_name() {
|
||||
declare desc="verify that the app name matches naming restrictions"
|
||||
local APP="$1"
|
||||
|
||||
@@ -6,7 +6,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"github.com/dokku/dokku/plugins/common"
|
||||
"mvdan.cc/sh/v3/shell"
|
||||
"mvdan.cc/sh/v3/syntax"
|
||||
)
|
||||
|
||||
// DefaultProcessType is the sentinel process-type key used for options that
|
||||
@@ -15,19 +15,23 @@ import (
|
||||
const DefaultProcessType = "_default_"
|
||||
|
||||
// SplitOptionString shell-tokenizes input, groups tokens on flag boundaries,
|
||||
// and returns one re-serialized option per group. A docker-options subcommand
|
||||
// flag (currently just --process) that lands inside the option content -
|
||||
// because the user typed it after the app name, where pflag's
|
||||
// SetInterspersed(false) hands it back as positional - is lifted into the
|
||||
// returned processes slice rather than stored as a docker option. The caller
|
||||
// merges those processes with whatever pflag already captured. Empty or
|
||||
// whitespace-only input returns empty slices.
|
||||
// and returns one re-serialized option per group. Tokenization honors quotes
|
||||
// for word boundaries but performs no expansion, so parameter expansions,
|
||||
// command substitutions, and other shell metacharacters are stored verbatim
|
||||
// and passed through to the container as written (the bash scheduler splits
|
||||
// them the same way, without expansion). A docker-options subcommand flag
|
||||
// (currently just --process) that lands inside the option content - because
|
||||
// the user typed it after the app name, where pflag's SetInterspersed(false)
|
||||
// hands it back as positional - is lifted into the returned processes slice
|
||||
// rather than stored as a docker option. The caller merges those processes
|
||||
// with whatever pflag already captured. Empty or whitespace-only input returns
|
||||
// empty slices.
|
||||
func SplitOptionString(input string) (options []string, processes []string, err error) {
|
||||
if strings.TrimSpace(input) == "" {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
fields, err := shell.Fields(input, func(string) string { return "" })
|
||||
fields, err := literalFields(input)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("Unable to parse docker option: %s", err.Error())
|
||||
}
|
||||
@@ -77,6 +81,79 @@ func SplitOptionString(input string) (options []string, processes []string, err
|
||||
return options, processes, nil
|
||||
}
|
||||
|
||||
// literalFields splits input into shell words using the parser directly, so
|
||||
// quotes delimit words and are stripped from the stored value, but nothing is
|
||||
// expanded. Parameter expansions, command substitutions, and other
|
||||
// metacharacters are preserved verbatim. Malformed input, such as an unbalanced
|
||||
// quote, returns the parser error.
|
||||
func literalFields(input string) ([]string, error) {
|
||||
parser := syntax.NewParser()
|
||||
var fields []string
|
||||
for word, err := range parser.WordsSeq(strings.NewReader(input)) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
fields = append(fields, literalWordValue(input, word.Parts))
|
||||
}
|
||||
return fields, nil
|
||||
}
|
||||
|
||||
// literalWordValue reconstructs the unquoted, unexpanded value of a shell word.
|
||||
// Literal and single-quoted parts contribute their literal text; double-quoted
|
||||
// parts have their surrounding quotes dropped while their contents stay
|
||||
// literal; every other part - parameter expansions, command substitutions,
|
||||
// arithmetic expansions - contributes its original source text unchanged.
|
||||
// Backslash escapes are removed the same way the shell removes them during
|
||||
// quote removal so a value such as `Host(\`app\`)` round-trips to `Host(`app`)`.
|
||||
func literalWordValue(input string, parts []syntax.WordPart) string {
|
||||
var sb strings.Builder
|
||||
for _, part := range parts {
|
||||
switch p := part.(type) {
|
||||
case *syntax.Lit:
|
||||
sb.WriteString(unquoteBackslashes(p.Value, false))
|
||||
case *syntax.SglQuoted:
|
||||
sb.WriteString(p.Value)
|
||||
case *syntax.DblQuoted:
|
||||
for _, inner := range p.Parts {
|
||||
if lit, ok := inner.(*syntax.Lit); ok {
|
||||
sb.WriteString(unquoteBackslashes(lit.Value, true))
|
||||
continue
|
||||
}
|
||||
sb.WriteString(input[inner.Pos().Offset():inner.End().Offset()])
|
||||
}
|
||||
default:
|
||||
sb.WriteString(input[part.Pos().Offset():part.End().Offset()])
|
||||
}
|
||||
}
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// unquoteBackslashes removes backslashes the way the shell does during quote
|
||||
// removal. Inside double quotes only \$, \`, \", \\, and an escaped newline
|
||||
// lose their backslash; unquoted, a backslash escapes any following character.
|
||||
func unquoteBackslashes(s string, inDblQuotes bool) string {
|
||||
if !strings.Contains(s, "\\") {
|
||||
return s
|
||||
}
|
||||
var sb strings.Builder
|
||||
for i := 0; i < len(s); i++ {
|
||||
if s[i] == '\\' && i+1 < len(s) {
|
||||
next := s[i+1]
|
||||
if next == '\n' {
|
||||
i++
|
||||
continue
|
||||
}
|
||||
if !inDblQuotes || next == '$' || next == '`' || next == '"' || next == '\\' {
|
||||
sb.WriteByte(next)
|
||||
i++
|
||||
continue
|
||||
}
|
||||
}
|
||||
sb.WriteByte(s[i])
|
||||
}
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// isFlagToken reports whether tok looks like a CLI flag (long or short) rather
|
||||
// than a value. Treating any token that begins with `-` and has more than one
|
||||
// character as a flag matches docker's flag conventions and avoids the need
|
||||
|
||||
@@ -96,6 +96,41 @@ func TestSplitOptionString(t *testing.T) {
|
||||
input: `--build-arg FOO="bar`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "command substitution is stored verbatim",
|
||||
input: "--label x=$(id)",
|
||||
wantOptions: []string{"--label 'x=$(id)'"},
|
||||
},
|
||||
{
|
||||
name: "backtick command substitution is stored verbatim",
|
||||
input: "--label x=`id`",
|
||||
wantOptions: []string{"--label 'x=`id`'"},
|
||||
},
|
||||
{
|
||||
name: "parameter expansion is stored verbatim",
|
||||
input: "--label x=$FOO",
|
||||
wantOptions: []string{"--label 'x=$FOO'"},
|
||||
},
|
||||
{
|
||||
name: "metacharacter value that parses is shell-quoted",
|
||||
input: `--label 'a;b'`,
|
||||
wantOptions: []string{"--label 'a;b'"},
|
||||
},
|
||||
{
|
||||
name: "escaped backtick inside double quotes is unescaped",
|
||||
input: "--label \"traefik.rule=Host(\\`app.example.com\\`)\"",
|
||||
wantOptions: []string{"--label 'traefik.rule=Host(`app.example.com`)'"},
|
||||
},
|
||||
{
|
||||
name: "escaped dollar inside double quotes is unescaped",
|
||||
input: "--label \"k=a\\$b\"",
|
||||
wantOptions: []string{"--label 'k=a$b'"},
|
||||
},
|
||||
{
|
||||
name: "unquoted backslash escape is removed",
|
||||
input: "--label k=a\\ b",
|
||||
wantOptions: []string{"--label 'k=a b'"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
@@ -351,3 +351,103 @@ func readLegacyOptionsFile(path string) ([]string, error) {
|
||||
}
|
||||
return lines, nil
|
||||
}
|
||||
|
||||
// traefikLabelMigrationKey gates the one-time pass that repairs docker-option
|
||||
// labels whose backticks were stored with a stray leading backslash by an
|
||||
// earlier release's option tokenizer (e.g. Traefik rules like Host(\`app\`)).
|
||||
const traefikLabelMigrationKey = "migrated-traefik-backticks"
|
||||
|
||||
// migrateTraefikLabelBackticks repairs stored Traefik label options whose
|
||||
// backticks carry a stray backslash (Host(\`app\`) instead of Host(`app`)) so
|
||||
// they become valid on the next deploy. Only Traefik --label options (those
|
||||
// whose label key begins with "traefik.") are considered, and only when they
|
||||
// contain a backslash-escaped backtick, which correct storage never produces.
|
||||
// It runs once, guarded by a global property.
|
||||
func migrateTraefikLabelBackticks() error {
|
||||
if common.PropertyGet("docker-options", "--global", traefikLabelMigrationKey) == "true" {
|
||||
return nil
|
||||
}
|
||||
|
||||
apps, err := common.DokkuApps()
|
||||
if err != nil {
|
||||
if errors.Is(err, common.NoAppsExist) {
|
||||
return common.PropertyWrite("docker-options", "--global", traefikLabelMigrationKey, "true")
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
for _, appName := range apps {
|
||||
properties, err := common.PropertyGetAll("docker-options", appName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for key := range properties {
|
||||
processType, phase, ok := splitPropertyKey(key)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
|
||||
options, err := GetDockerOptionsForProcessPhase(appName, processType, phase)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
changed := false
|
||||
for i, option := range options {
|
||||
if fixed, ok := repairTraefikLabelBackticks(option); ok {
|
||||
options[i] = fixed
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
if changed {
|
||||
if err := writeDockerOptionsForProcessPhase(appName, processType, phase, options); err != nil {
|
||||
return err
|
||||
}
|
||||
common.LogInfo1(fmt.Sprintf("Repaired docker-options label backticks for %s (%s %s)", appName, processType, phase))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return common.PropertyWrite("docker-options", "--global", traefikLabelMigrationKey, "true")
|
||||
}
|
||||
|
||||
// repairTraefikLabelBackticks removes the stray backslash before a backtick in
|
||||
// a Traefik --label option (one whose label key begins with "traefik."),
|
||||
// returning the repaired option and whether anything changed. Backticks are
|
||||
// Traefik's rule syntax, so the repair is scoped to Traefik labels; non-label
|
||||
// options, non-Traefik labels, and labels without a backslash-escaped backtick
|
||||
// are left untouched so an intentional backslash elsewhere is never rewritten.
|
||||
func repairTraefikLabelBackticks(option string) (string, bool) {
|
||||
spec, ok := labelSpec(option)
|
||||
if !ok || !strings.HasPrefix(spec, "traefik.") || !strings.Contains(option, "\\`") {
|
||||
return option, false
|
||||
}
|
||||
return strings.ReplaceAll(option, "\\`", "`"), true
|
||||
}
|
||||
|
||||
// labelSpec returns the "key=value" portion of a stored --label/-l option with
|
||||
// the flag and any surrounding shell-quoting removed so the label key can be
|
||||
// inspected. It reports ok=false for options that do not set a docker label.
|
||||
func labelSpec(option string) (string, bool) {
|
||||
s := strings.TrimLeft(option, " ")
|
||||
if len(s) >= 2 && s[0] == '\'' && s[len(s)-1] == '\'' {
|
||||
s = s[1 : len(s)-1]
|
||||
}
|
||||
switch {
|
||||
case strings.HasPrefix(s, "--label="):
|
||||
s = s[len("--label="):]
|
||||
case strings.HasPrefix(s, "--label "):
|
||||
s = s[len("--label "):]
|
||||
case strings.HasPrefix(s, "-l="):
|
||||
s = s[len("-l="):]
|
||||
case strings.HasPrefix(s, "-l "):
|
||||
s = s[len("-l "):]
|
||||
default:
|
||||
return "", false
|
||||
}
|
||||
s = strings.TrimPrefix(s, "'")
|
||||
s = strings.TrimPrefix(s, "\"")
|
||||
return s, true
|
||||
}
|
||||
|
||||
@@ -226,6 +226,58 @@ func TestMigrateLegacyDockerOptionsFiles_SkipsEmptyContentFiles(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepairTraefikLabelBackticks(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
changed bool
|
||||
}{
|
||||
{"traefik rule space form", "--label 'traefik.http.routers.web.rule=Host(\\`app.example.com\\`)'", "--label 'traefik.http.routers.web.rule=Host(`app.example.com`)'", true},
|
||||
{"traefik rule whole-quoted equals form", "'--label=traefik.http.routers.web.rule=Host(\\`x\\`)'", "'--label=traefik.http.routers.web.rule=Host(`x`)'", true},
|
||||
{"non-traefik label left untouched", "--label 'some.key=Host(\\`x\\`)'", "--label 'some.key=Host(\\`x\\`)'", false},
|
||||
{"already valid traefik label", "--label 'traefik.rule=Host(`x`)'", "--label 'traefik.rule=Host(`x`)'", false},
|
||||
{"non-label option left untouched", "-v /tmp", "-v /tmp", false},
|
||||
{"traefik label without backticks", "--label 'traefik.enable=true'", "--label 'traefik.enable=true'", false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got, changed := repairTraefikLabelBackticks(tc.in)
|
||||
if got != tc.want || changed != tc.changed {
|
||||
t.Errorf("repairTraefikLabelBackticks(%q) = (%q, %v), want (%q, %v)", tc.in, got, changed, tc.want, tc.changed)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMigrateTraefikLabelBackticks(t *testing.T) {
|
||||
dokkuRoot := setupMigrationEnv(t)
|
||||
if err := os.MkdirAll(filepath.Join(dokkuRoot, "alpha"), 0755); err != nil {
|
||||
t.Fatalf("MkdirAll: %v", err)
|
||||
}
|
||||
|
||||
broken := "--label 'traefik.http.routers.web.rule=Host(\\`app.example.com\\`)'"
|
||||
if err := common.PropertyListWrite("docker-options", "alpha", "_default_.deploy", []string{broken}); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
if err := migrateTraefikLabelBackticks(); err != nil {
|
||||
t.Fatalf("migrateTraefikLabelBackticks: %v", err)
|
||||
}
|
||||
|
||||
got, err := GetDockerOptionsForProcessPhase("alpha", "_default_", "deploy")
|
||||
if err != nil {
|
||||
t.Fatalf("GetDockerOptionsForProcessPhase: %v", err)
|
||||
}
|
||||
want := "--label 'traefik.http.routers.web.rule=Host(`app.example.com`)'"
|
||||
if !equalStrings(got, []string{want}) {
|
||||
t.Errorf("got %q, want [%q]", got, []string{want})
|
||||
}
|
||||
if common.PropertyGet("docker-options", "--global", traefikLabelMigrationKey) != "true" {
|
||||
t.Errorf("expected %s guard to be set", traefikLabelMigrationKey)
|
||||
}
|
||||
}
|
||||
|
||||
func equalStrings(a, b []string) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
|
||||
@@ -20,6 +20,10 @@ func TriggerInstall() error {
|
||||
return fmt.Errorf("Unable to migrate legacy docker-options files: %v", err)
|
||||
}
|
||||
|
||||
if err := migrateTraefikLabelBackticks(); err != nil {
|
||||
return fmt.Errorf("Unable to repair docker-options label backticks: %v", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -86,6 +86,10 @@ fn-run() {
|
||||
DOKKU_RUN_TTL_SECONDS="86400"
|
||||
fi
|
||||
|
||||
if ! is_number "$DOKKU_RUN_TTL_SECONDS"; then
|
||||
dokku_log_fail "--ttl-seconds must be a positive integer"
|
||||
fi
|
||||
|
||||
if [[ "$CMD" == "run:detached" ]] && [[ "$DOKKU_FORCE_TTY" != "true" ]]; then
|
||||
export DOKKU_DISABLE_TTY=true
|
||||
fi
|
||||
|
||||
@@ -54,9 +54,10 @@ fn-scheduler-deploy-process() {
|
||||
local DOCKER_ARGS
|
||||
DOCKER_ARGS=$(: | plugn trigger docker-args-deploy "$APP" "$IMAGE_TAG" "$PROC_TYPE")
|
||||
DOCKER_ARGS+=" $(: | plugn trigger docker-args-process-deploy "$APP" "$IMAGE_SOURCE_TYPE" "$IMAGE_TAG" "$PROC_TYPE")"
|
||||
DOCKER_ARGS=" $DOCKER_ARGS "
|
||||
declare -a ARG_ARRAY
|
||||
eval "ARG_ARRAY=($DOCKER_ARGS)"
|
||||
declare -a ARG_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
ARG_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
|
||||
local port_published=false
|
||||
for arg in "${ARG_ARRAY[@]}"; do
|
||||
|
||||
@@ -67,8 +67,10 @@ main() {
|
||||
DOCKER_ARGS+=" $IMAGE"
|
||||
DOCKER_ARGS+=" $START_CMD"
|
||||
|
||||
declare -a ARG_ARRAY
|
||||
eval "ARG_ARRAY=($DOCKER_ARGS)"
|
||||
declare -a ARG_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
ARG_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
cid=$(fn-scheduler-docker-local-start-app-container "$APP" "${ARG_ARRAY[@]}")
|
||||
|
||||
plugn trigger post-container-create "app" "$cid" "$APP" "deploy" "$PROC_TYPE"
|
||||
|
||||
@@ -133,8 +133,10 @@ trigger-scheduler-docker-local-scheduler-run() {
|
||||
DOCKER_ARGS+=" $IMAGE"
|
||||
DOCKER_ARGS+=" $EXEC_CMD"
|
||||
|
||||
declare -a ARG_ARRAY
|
||||
eval "ARG_ARRAY=($DOCKER_ARGS)"
|
||||
declare -a ARG_ARRAY=()
|
||||
while IFS= read -r -d '' arg; do
|
||||
ARG_ARRAY+=("$arg")
|
||||
done < <(fn-docker-args-split "$DOCKER_ARGS")
|
||||
|
||||
RUN_COMMAND=("$@")
|
||||
if [[ "${#RUN_COMMAND[@]}" -eq 0 ]]; then
|
||||
|
||||
@@ -86,14 +86,14 @@ trigger-traefik-vhosts-docker-args-process-deploy() {
|
||||
if [[ -n "$app_domains" ]]; then
|
||||
# get length of domains
|
||||
if [[ "$(echo "$traefik_domains" | wc -w)" -eq 1 ]]; then
|
||||
traefik_domains="Host(\\\`$traefik_domains\\\`)"
|
||||
traefik_domains="Host(\`$traefik_domains\`)"
|
||||
else
|
||||
for domain in $(echo "$app_domains" | xargs); do
|
||||
if [[ -z "$traefik_domains" ]]; then
|
||||
traefik_domains="Host(\\\`$domain\\\`)"
|
||||
traefik_domains="Host(\`$domain\`)"
|
||||
continue
|
||||
fi
|
||||
traefik_domains="$traefik_domains || Host(\\\`$domain\\\`)"
|
||||
traefik_domains="$traefik_domains || Host(\`$domain\`)"
|
||||
done
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -105,6 +105,11 @@ teardown() {
|
||||
}
|
||||
|
||||
@test "(builder-herouish:build .env)" {
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP build '--label=com.dokku.build-test=safe'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
|
||||
@@ -146,6 +146,11 @@ teardown() {
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP build '--label=com.dokku.build-test=safe'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP add_requirements_txt
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
|
||||
@@ -178,6 +178,11 @@ teardown() {
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP build '--label=com.dokku.build-test=safe'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP add_requirements_txt_cnb
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
|
||||
@@ -157,6 +157,11 @@ teardown() {
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP build '--label=com.dokku.build-test=safe'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP add_requirements_txt
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
|
||||
@@ -91,3 +91,32 @@ teardown() {
|
||||
echo "status: $status"
|
||||
assert_output "false"
|
||||
}
|
||||
|
||||
@test "(common) fn-docker-args-split does not expand injected commands" {
|
||||
export DOKKU_TEST_PAYLOAD='--label x=$(id)'
|
||||
run /bin/bash -c "source '$PLUGIN_CORE_AVAILABLE_PATH/common/functions'; fn-docker-args-split \"\$DOKKU_TEST_PAYLOAD\" | tr '\\0' '\\n'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
assert_line 1 'x=$(id)'
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - value was expanded"
|
||||
|
||||
export DOKKU_TEST_PAYLOAD='--label x=`id`'
|
||||
run /bin/bash -c "source '$PLUGIN_CORE_AVAILABLE_PATH/common/functions'; fn-docker-args-split \"\$DOKKU_TEST_PAYLOAD\" | tr '\\0' '\\n'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
assert_line 1 'x=`id`'
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - value was expanded"
|
||||
|
||||
unset DOKKU_TEST_PAYLOAD
|
||||
}
|
||||
|
||||
@test "(common) fn-docker-args-split preserves quoted tokens" {
|
||||
run /bin/bash -c "source '$PLUGIN_CORE_AVAILABLE_PATH/common/functions'; fn-docker-args-split \"--label 'a b'\" | tr '\\0' '\\n'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
assert_line 0 "--label"
|
||||
assert_line 1 "a b"
|
||||
}
|
||||
|
||||
@@ -285,6 +285,25 @@ teardown() {
|
||||
assert_output "/tmp:{} /var/tmp:{}"
|
||||
}
|
||||
|
||||
@test "(docker-options) deploy does not expand command substitution in option values [buildpacks]" {
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP deploy '--label=com.dokku.test=\$(id)'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run deploy_app
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
CID=$(<$DOKKU_ROOT/$TEST_APP/CONTAINER.web.1)
|
||||
run /bin/bash -c "docker inspect $CID --format '{{ index .Config.Labels \"com.dokku.test\" }}'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_output '$(id)'
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - option value was expanded"
|
||||
}
|
||||
|
||||
@test "(docker-options) deploy with options [dockerfile]" {
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP deploy \"-v /var/tmp\""
|
||||
echo "output: $output"
|
||||
|
||||
@@ -153,3 +153,46 @@ teardown() {
|
||||
assert_success
|
||||
assert_output "2"
|
||||
}
|
||||
|
||||
@test "(run) docker-options and -e flags are not eval-injected" {
|
||||
run deploy_app
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP run '--label=com.dokku.test=safe'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku run --ttl-seconds=60 -e FOO=bar $TEST_APP env | grep -E '^FOO=bar'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku run -e 'X=\$(id)' $TEST_APP env"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
[[ "$output" == *'X=$(id)'* ]] || flunk "expected literal X=\$(id) in container env"
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - env value was expanded"
|
||||
}
|
||||
|
||||
@test "(run) --ttl-seconds rejects non-numeric input; docker-options stores it verbatim" {
|
||||
run /bin/bash -c "dokku run --ttl-seconds '\$(id)' $TEST_APP echo hi"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_failure
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - ttl value was expanded"
|
||||
|
||||
run /bin/bash -c "dokku docker-options:add $TEST_APP run '--label=x=\$(id)'"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
assert_success
|
||||
|
||||
run /bin/bash -c "dokku docker-options:report $TEST_APP --docker-options-run"
|
||||
echo "output: $output"
|
||||
echo "status: $status"
|
||||
[[ "$output" == *'x=$(id)'* ]] || flunk "expected literal x=\$(id) stored in docker options"
|
||||
[[ "$output" != *"uid="* ]] || flunk "id command output leaked - docker option was expanded"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user