Преглед изворни кода

fix(docs): correct shellAfterCompleted quoting for output and exitCode
The after-completion example put {{ output }} and {{ exitCode }} inside
single-quoted shell arguments, so and never expanded
after substituteShellAfterCompletedEnvRefs runs.
Update the apprise sample to use printf-based quoting that allows env
substitution, document the single-quote pitfall, and extend
TestShellAfterCompletedUsesOutputEnvSafely to assert stdout is
substituted as well as injection being blocked.

jamesread пре 2 месеци
родитељ
комит
8eda67a39e

+ 2 - 2
docs/modules/ROOT/pages/action_execution/aftercompletion.adoc

@@ -10,13 +10,13 @@ actions:
   - title: Check date and send notification via apprise
     icon: date
     shell: date
-    shellAfterCompleted: "apprise -c /config/apprise.yml -t 'Notification: Backup script completed' -b 'The backup script completed with code {{ exitCode}}. The log is: \n {{ output }} '"
+    shellAfterCompleted: "apprise -c /config/apprise.yml -t 'Notification: Backup script completed' -b \"$(printf 'Backup completed with exit code %s. Log: %s' {{ exitCode }} {{ output }})\""
 ----
 
 When running shellAfterCompleted, you *cannot* use argument values - they are not passed to the command. However the following special arguments are defined;
 
 * `{{ exitCode }}` - The exit code of the previous shell command. OliveTin substitutes this with the `EXITCODE` environment variable when running `shellAfterCompleted`, so shell metacharacters in the value cannot break quoting.
-* `{{ output }}` - The standard output of the previous shell command. OliveTin substitutes this with the `OUTPUT` environment variable when running `shellAfterCompleted`, so shell metacharacters in command output cannot be executed. You can also reference `$OUTPUT` directly in your `shellAfterCompleted` command.
+* `{{ output }}` - The standard output of the previous shell command. OliveTin substitutes this with the `OUTPUT` environment variable when running `shellAfterCompleted`, so shell metacharacters in command output cannot be executed. You can also reference `$OUTPUT` directly in your `shellAfterCompleted` command. Do not place these placeholders inside single-quoted shell arguments; single quotes prevent `$OUTPUT` and `$EXITCODE` from expanding after substitution.
 * `{{ .Arguments.ot_executionTrackingId }}` - The unique execution tracking id for this execution (version 3k; in 2k use `{{ ot_executionTrackingId }}`)
 * `{{ .Arguments.ot_username }}` - The username of the user who started the execution (version 3k; in 2k use `{{ ot_username }}`). May be `guest` or `cron` for unauthenticated or automated runs.
 

+ 5 - 2
service/internal/executor/executor_test.go

@@ -421,10 +421,11 @@ func TestShellAfterCompletedUsesOutputEnvSafely(t *testing.T) {
 	cfg := config.DefaultConfig()
 	e := DefaultExecutor(cfg)
 	injectedPath := filepath.Join(t.TempDir(), "olivetin-injected")
+	expectedMainOutput := "'; touch " + injectedPath + "; echo '"
 	a1 := &config.Action{
 		Title:               "After completion escape",
-		Shell:               "printf %s \"'; touch " + injectedPath + "; echo '\"",
-		ShellAfterCompleted: "printf %s '{{ output }}'",
+		Shell:               "printf %s \"" + expectedMainOutput + "\"",
+		ShellAfterCompleted: "printf %s {{ output }}",
 	}
 	cfg.Actions = append(cfg.Actions, a1)
 	cfg.Sanitize()
@@ -441,6 +442,8 @@ func TestShellAfterCompletedUsesOutputEnvSafely(t *testing.T) {
 
 	assert.NotNil(t, req.logEntry)
 	assert.Equal(t, int32(0), req.logEntry.ExitCode)
+	assert.True(t, strings.HasPrefix(req.logEntry.Output, expectedMainOutput))
+	assert.Contains(t, req.logEntry.Output, "OliveTin::shellAfterCompleted stdout\n"+expectedMainOutput)
 	_, err := os.Stat(injectedPath)
 	assert.True(t, os.IsNotExist(err), "shellAfterCompleted must not execute injected commands from output")
 }