From 9e7a397898bb5c905e12d478469a876243d9eccf Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 10:48:32 +0100 Subject: [PATCH 01/21] fix: Action title styling on the argument form --- frontend/resources/vue/views/ArgumentForm.vue | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/frontend/resources/vue/views/ArgumentForm.vue b/frontend/resources/vue/views/ArgumentForm.vue index 9da632c..5da2f6d 100644 --- a/frontend/resources/vue/views/ArgumentForm.vue +++ b/frontend/resources/vue/views/ArgumentForm.vue @@ -4,17 +4,12 @@

Start action: - - - {{ title }} - + + {{ title }}

@@ -684,18 +679,6 @@ onUnmounted(() => { font-size: 1.5rem; } -.action-details-title-link { - display: inline-flex; - align-items: center; - gap: 0.5rem; - color: var(--link-color, #0066cc); - text-decoration: underline; -} - -.action-details-title-link:hover { - color: var(--link-hover-color, #004499); -} - form { grid-template-columns: max-content auto auto; } From 663103ad0a6689bb20a24a3b9c23bfc9ab895d47 Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 11:13:41 +0100 Subject: [PATCH 02/21] chore: better column ordering in the diagnostics view config issues --- frontend/resources/vue/views/DiagnosticsView.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/resources/vue/views/DiagnosticsView.vue b/frontend/resources/vue/views/DiagnosticsView.vue index 4c68e44..cfb47f8 100644 --- a/frontend/resources/vue/views/DiagnosticsView.vue +++ b/frontend/resources/vue/views/DiagnosticsView.vue @@ -125,11 +125,11 @@ const browserInfoCopied = ref(false) const configIssueHeaders = computed(() => [ { key: 'severity', label: t('diagnostics.config-issue-severity'), sortable: true, width: '7rem' }, + { key: 'configFile', label: t('diagnostics.config-issue-config-file'), sortable: true, width: '14rem' }, { key: 'code', label: t('diagnostics.config-issue-code'), sortable: true, width: '12rem' }, { key: 'message', label: t('diagnostics.config-issue-message'), sortable: false }, { key: 'actionTitle', label: t('diagnostics.config-issue-action'), sortable: true, width: '10rem' }, { key: 'argumentName', label: t('diagnostics.config-issue-argument'), sortable: true, width: '8rem' }, - { key: 'configFile', label: t('diagnostics.config-issue-config-file'), sortable: true, width: '14rem' }, { key: 'source', label: t('diagnostics.config-issue-source'), sortable: false, width: '12rem' } ]) From 07f7e12991269c3fe656d4614399cbca75921e88 Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 28 Jul 2026 12:47:55 +0100 Subject: [PATCH 03/21] fix: Logs page now remembers filters between page navigation --- .../resources/vue/views/LogsCalendarView.vue | 16 ++++++++++-- frontend/resources/vue/views/LogsListView.vue | 25 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/frontend/resources/vue/views/LogsCalendarView.vue b/frontend/resources/vue/views/LogsCalendarView.vue index ed6d517..ceb0b81 100644 --- a/frontend/resources/vue/views/LogsCalendarView.vue +++ b/frontend/resources/vue/views/LogsCalendarView.vue @@ -5,7 +5,7 @@ > diff --git a/frontend/resources/vue/components/DashboardComponentDisplay.vue b/frontend/resources/vue/components/DashboardComponentDisplay.vue index 6450325..daf0867 100644 --- a/frontend/resources/vue/components/DashboardComponentDisplay.vue +++ b/frontend/resources/vue/components/DashboardComponentDisplay.vue @@ -3,7 +3,9 @@ class="display" :class="component.cssClass" > +
+
diff --git a/frontend/resources/vue/views/ArgumentForm.vue b/frontend/resources/vue/views/ArgumentForm.vue index 5da2f6d..d086d1d 100644 --- a/frontend/resources/vue/views/ArgumentForm.vue +++ b/frontend/resources/vue/views/ArgumentForm.vue @@ -89,10 +89,12 @@ @change="handleChange(arg, $event)" /> + + diff --git a/frontend/resources/vue/views/ExecutionView.vue b/frontend/resources/vue/views/ExecutionView.vue index 8746e65..b254d82 100644 --- a/frontend/resources/vue/views/ExecutionView.vue +++ b/frontend/resources/vue/views/ExecutionView.vue @@ -58,7 +58,7 @@ >
Duration
-
+
{{ duration }}
Status
@@ -373,7 +373,7 @@ function updateDuration (logEntryParam) { } catch (e) { console.warn('Failed to calculate delta', e) } - duration.value = logEntry.value.datetimeStarted + ' → ' + logEntry.value.datetimeFinished + duration.value = logEntry.value.datetimeStarted + ' → ' + logEntry.value.datetimeFinished if (delta !== '') { duration.value += ' (' + delta + ')' } diff --git a/specs/config-issues.md b/specs/config-issues.md new file mode 100644 index 0000000..2a996c3 --- /dev/null +++ b/specs/config-issues.md @@ -0,0 +1,50 @@ +# Configuration issues + +This spec describes how OliveTin collects configuration warnings and errors and surfaces them in the web UI. + +## Purpose + +Operators should see configuration problems in Diagnostics instead of only in server logs. When any issues exist, the Diagnostics navigation link shows a count badge. + +## When issues are rebuilt + +The issue list is cleared and rebuilt when the action map is rebuilt. That happens after configuration load or reload, and after entity data changes. Some load-time findings that cannot be re-derived after decode (for example unset environment variables already expanded away) are kept across rebuilds until the next configuration load begins. + +## What is collected + +Issues include: + +- Unknown or unenforced action group references +- Checklist arguments with missing or invalid choice templates +- Arguments whose type was left unset (defaulted to ascii) +- Unset environment variables referenced from configuration +- Missing or invalid include directories +- Argument default or choice templates that fail to parse +- Literal argument defaults that fail type validation (templated defaults are not type-checked as raw text) +- Entity files that cannot be read or parsed, or are empty +- Entity-bound actions with no entity instances (after OliveTin has attempted to load that entity type, so startup does not report a false positive before entity files are read) +- Invalid cron schedules +- Entity-bound actions that also use scheduled cron execution (cron runs without an entity binding and will not execute) +- Filesystem watch paths that cannot be created (missing directories for file-in-dir triggers, calendar files, or entity files). Runtime watcher setup failures for action triggers include the related action so view permissions still apply; entity-file watchers without an action remain visible to anyone who may view Diagnostics. + +Each issue has a severity of warning or error, a stable code, a human-readable message, and optional context such as action title, argument name, configuration source file, or detail value. + +When the issue list is rebuilt, OliveTin logs only newly appeared issues so startup does not repeat the same warning for every action-map rebuild. + +When configuration is loaded from a base file and an include directory, OliveTin records which file defined each action and entity declaration. That path is shown as the configuration source file when available. Some issues (for example unset environment variables) may not have a specific file. Entity data file problems also show the entity data path in the detail column. + +## Diagnostics page + +Users who are allowed to view Diagnostics see a Configuration issues section listing the current issues in a table. When there are none, the section states that no configuration issues were detected. + +Action-scoped issues are only included when the user is allowed to view that action. Issues that are not tied to an action (for example unset environment variables or missing include directories) remain visible to anyone who may view Diagnostics. + +Users who are not allowed to view Diagnostics cannot retrieve the issue list. + +## Navigation count + +When Diagnostics is visible and at least one configuration issue exists that the user is allowed to see, the Diagnostics navigation link shows a count badge with the number of those issues. The badge clears when the visible issue count becomes zero after a configuration or entity refresh. + +## Init count + +The Init response includes the configuration issue count for users who may view Diagnostics, using the same per-user filtering as the Diagnostics list. For other users the count is zero. diff --git a/specs/dashboard-nav-categories.md b/specs/dashboard-nav-categories.md new file mode 100644 index 0000000..671c615 --- /dev/null +++ b/specs/dashboard-nav-categories.md @@ -0,0 +1,35 @@ +# Spec: Dashboard navigation categories + +This spec describes how root dashboards can be grouped into categories in the sidebar navigation. + +--- + +## 1. Configuration + +Root dashboard entries in the configuration may include an optional category label. + +- The category applies only to root dashboards (top-level items in the dashboards list). Nested dashboard contents ignore category. +- If category is omitted or empty, the dashboard is uncategorized. +- Dashboards that share the same category label are grouped together under that label in the sidebar. + +## 2. Visibility + +Only dashboards the current user is allowed to view appear in navigation. + +- Access-denied dashboards are omitted from the list and do not create empty category sections. +- If every dashboard in a category is hidden, that category does not appear. + +## 3. Sidebar ordering + +When building the sidebar: + +1. Uncategorized dashboards appear first, as a flat list above any category sections, in configuration order among visible uncategorized dashboards. +2. Category sections follow, in the order each category first appears among visible categorized dashboards. +3. Within a category, dashboards keep the order they appear in the configuration among visible dashboards in that category. +4. After all dashboard links, a **System** category lists Entities, Logs, and Diagnostics (each only when the user is allowed to see that item). If none of those links are visible, the System category is omitted. + +The default Actions dashboard, when present, is uncategorized unless configuration gives it a category (it is not a configured root entry, so it stays uncategorized). + +## 4. Navigation style + +Category sections apply when section navigation uses the sidebar. Top-bar navigation does not show category section headers; dashboard links still appear in the same relative order without collapsible category groups. From 1739f1e127fee47259002c525a5059436e5fcd29 Mon Sep 17 00:00:00 2001 From: jamesread Date: Wed, 29 Jul 2026 01:07:04 +0100 Subject: [PATCH 20/21] chore: removed warnings from default config --- config.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config.yaml b/config.yaml index 9ee2c2d..19da832 100644 --- a/config.yaml +++ b/config.yaml @@ -74,7 +74,7 @@ actions: icon: backup onclick: execution-dialog # https://docs.olivetin.app/action_execution/oncalendar.html - execOnCalendarFile: examples/demo-olivetin-calendar.yaml + # execOnCalendarFile: examples/demo-olivetin-calendar.yaml - title: Verify backup archive shell: sleep 3 && echo "Backup archive verified" @@ -146,7 +146,7 @@ actions: title: Are you sure?! # Checklist arguments let users pick multiple predefined options. Selected - # values are passed to the action as a comma-separated string. + # values are passed to the action as a JSON array string. # # Docs: https://docs.olivetin.app/args/input_checklist.html - title: Backup selected directories @@ -166,7 +166,7 @@ actions: value: music - title: Videos value: videos - default: documents,photos + default: '["documents","photos"]' # This is an action that runs a script included with OliveTin, that will # download themes. You will still need to set theme "themeName" in your config. @@ -263,8 +263,8 @@ actions: icon: ping # https://docs.olivetin.app/action_execution/onfilecreated.html # mkdir -p /tmp/olivetin-demo-file-created - execOnFileCreatedInDir: - - /tmp/olivetin-demo-file-created + # execOnFileCreatedInDir: + # - /tmp/olivetin-demo-file-created - title: Start {{ .CurrentEntity.Names }} icon: box From 0b2a5995ff62db23060608746cf6e53e0abace12 Mon Sep 17 00:00:00 2001 From: jamesread Date: Wed, 29 Jul 2026 01:25:44 +0100 Subject: [PATCH 21/21] chore: coderabbit knows better than me --- Makefile | 2 +- .../action_execution/aftercompletion.adoc | 4 +- .../pages/action_execution/onwebhook.adoc | 10 ++- service/.golangci.yml | 40 +++++++++-- service/internal/api/api.go | 5 +- service/internal/api/api_test.go | 13 ++-- service/internal/auth/otjwt/jwt_test.go | 20 ++---- .../auth/otoauth2/restapi_auth_oauth2.go | 2 +- service/internal/executor/arguments_test.go | 2 +- service/internal/executor/executor.go | 71 +++++++++++++++++-- service/internal/executor/executor_test.go | 55 ++++++++++++-- specs/config-issues.md | 10 +-- specs/dashboard-nav-categories.md | 2 +- 13 files changed, 186 insertions(+), 50 deletions(-) diff --git a/Makefile b/Makefile index b468f74..3ec7211 100644 --- a/Makefile +++ b/Makefile @@ -79,4 +79,4 @@ config-tool: devcheck: python3 scripts/devcheck.py $(ARGS) -.PHONY: proto service windows-resources windows-msi devcheck +.PHONY: proto service windows-resources windows-msi frontend-unittests devcheck diff --git a/docs/modules/ROOT/pages/action_execution/aftercompletion.adoc b/docs/modules/ROOT/pages/action_execution/aftercompletion.adoc index 08a5211..c598a87 100644 --- a/docs/modules/ROOT/pages/action_execution/aftercompletion.adoc +++ b/docs/modules/ROOT/pages/action_execution/aftercompletion.adoc @@ -15,8 +15,8 @@ actions: When running shellAfterCompleted, you *cannot* use user-defined argument values - they are not passed to the command. However the following special arguments are defined; -* `{{ exitCode }}` / `{{ .Arguments.exitCode }}` - The exit code of the previous command. OliveTin rewrites these placeholders to the quoted `"$EXITCODE"` environment reference when running `shellAfterCompleted`, so shell metacharacters in the value cannot break quoting. -* `{{ output }}` / `{{ .Arguments.output }}` - The standard output of the previous command. OliveTin rewrites these placeholders to the quoted `"$OUTPUT"` environment reference, 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. +* `{{ exitCode }}` / `{{ .Arguments.exitCode }}` - The exit code of the previous command. OliveTin rewrites these placeholders to the quoted `"$EXITCODE"` environment reference when running `shellAfterCompleted`, so shell metacharacters in the value cannot break quoting. Placeholders inside single-quoted shell arguments are rewritten so the environment reference can still expand. +* `{{ output }}` / `{{ .Arguments.output }}` - The standard output of the previous command. OliveTin rewrites these placeholders to the quoted `"$OUTPUT"` environment reference, so shell metacharacters in command output cannot be executed. You can also reference `$OUTPUT` directly in your `shellAfterCompleted` command. Placeholders inside single-quoted shell arguments are rewritten so the environment reference can still expand. * `{{ .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. diff --git a/docs/modules/ROOT/pages/action_execution/onwebhook.adoc b/docs/modules/ROOT/pages/action_execution/onwebhook.adoc index 4735025..3a81982 100644 --- a/docs/modules/ROOT/pages/action_execution/onwebhook.adoc +++ b/docs/modules/ROOT/pages/action_execution/onwebhook.adoc @@ -81,9 +81,11 @@ actions: - matchQuery: action: deploy env: production + extract: + service: "$.service" ---- -A request to `/webhooks?action=deploy&env=production` would match this action. +A request to `/webhooks?action=deploy&env=production` with a JSON body containing `"service"` would match this action and pass that field into the `service` argument. === Match by JSON Body Path @@ -101,6 +103,8 @@ actions: type: ascii execOnWebhook: - matchPath: "$.event_type=push" + extract: + branch: "$.branch" ---- The `matchPath` format is `jsonpath=value`. You can also just specify a JSONPath without a value to match if the path exists: @@ -190,7 +194,7 @@ actions: - title: Log Request exec: - echo - - "Request ID: {{ webhook_header_x-request-id }}" + - 'Request ID: {{ index .Arguments "webhook_header_x-request-id" }}' arguments: - name: webhook_header_x-request-id type: ascii @@ -199,6 +203,8 @@ actions: X-Event-Type: log ---- +Header names that contain hyphens become argument keys with the same hyphens (for example `webhook_header_x-request-id`). Use the `index` map-lookup form shown above, because Go templates treat hyphens in bare identifiers as subtraction. + == Webhook Authentication OliveTin supports several authentication methods to verify webhook requests: diff --git a/service/.golangci.yml b/service/.golangci.yml index 6f7462d..4298e10 100644 --- a/service/.golangci.yml +++ b/service/.golangci.yml @@ -53,7 +53,17 @@ linters: - path: scripts/ linters: - gosec - - path: cmd/ + # Local config-tool CLI: operator-supplied path and 0644 config backups. + - path: cmd/config-tool/main\.go + text: "G304:" + linters: + - gosec + - path: cmd/config-tool/main\.go + text: "G306:" + linters: + - gosec + - path: cmd/config-tool/main\.go + text: "G703:" linters: - gosec @@ -102,11 +112,33 @@ linters: - gosec # Secure is set dynamically from TLS / ForceSecureCookies; gosec wants a literal true. - - text: "G124:" + - path: internal/api/api\.go + text: "G124:" + linters: + - gosec + - path: internal/auth/otoauth2/ + text: "G124:" linters: - gosec - # Protobuf / process exit codes mapped into int32 fields. - - text: "G115:" + # Protobuf / process exit codes and collection sizes mapped into int32 fields. + - path: internal/api/apiActions\.go + text: "G115:" + linters: + - gosec + - path: internal/api/api_entities_list\.go + text: "G115:" + linters: + - gosec + - path: internal/api/api_queue\.go + text: "G115:" + linters: + - gosec + - path: internal/api/config_issues\.go + text: "G115:" + linters: + - gosec + - path: internal/executor/executor\.go + text: "G115:" linters: - gosec diff --git a/service/internal/api/api.go b/service/internal/api/api.go index 6b82e31..7fc3dc0 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -43,7 +43,6 @@ type oliveTinAPI struct { streamingClientsMutex sync.RWMutex } -// Caps concurrent EventStream connections to limit memory/FD/goroutine exhaustion. const maxEventStreamClients = 16 var errEventStreamClientLimit = errors.New("too many concurrent event stream clients") @@ -1360,8 +1359,8 @@ func (api *oliveTinAPI) addCustomDashboardEntries(entries *[]*apiv1.RootDashboar for _, dashboard := range dashboards { // We have to build the dashboard response instead of just looping over config.dashboards, // because we need to check if the user has access to the dashboard - db := renderDashboard(rr, dashboard.Title) - if db != nil { + renderedDashboard := renderDashboard(rr, dashboard.Title) + if renderedDashboard != nil { *entries = append(*entries, &apiv1.RootDashboard{ Title: dashboard.Title, Category: dashboard.Category, diff --git a/service/internal/api/api_test.go b/service/internal/api/api_test.go index ad86e2b..528b0d3 100644 --- a/service/internal/api/api_test.go +++ b/service/internal/api/api_test.go @@ -138,11 +138,11 @@ func TestGetEntities(t *testing.T) { resp, err := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{})) require.NoError(t, err, "GetEntities should not return an error") - assert.NotNil(t, resp, "GetEntities response should not be nil") - assert.NotNil(t, resp.Msg, "GetEntities response message should not be nil") + require.NotNil(t, resp, "GetEntities response should not be nil") + require.NotNil(t, resp.Msg, "GetEntities response message should not be nil") entityDefinitions := resp.Msg.EntityDefinitions - assert.Len(t, entityDefinitions, 3, "Should return 3 entity definitions") + require.Len(t, entityDefinitions, 3, "Should return 3 entity definitions") validateEntityOrderAndStructure(t, entityDefinitions) validateNoDuplicates(t, entityDefinitions) @@ -189,6 +189,8 @@ func setupTestEntities() { func validateEntityOrderAndStructure(t *testing.T, entityDefinitions []*apiv1.EntityDefinition) { t.Helper() + require.GreaterOrEqual(t, len(entityDefinitions), 3, "Need at least three entity definitions before indexing") + assert.Equal(t, "application", entityDefinitions[0].Title, "First entity should be 'application' (alphabetically first)") assert.Len(t, entityDefinitions[0].Instances, 1, "Application should have 1 instance") assert.Equal(t, "webapp", entityDefinitions[0].Instances[0].UniqueKey, "Application instance should be 'webapp'") @@ -221,11 +223,12 @@ func validateConsistency(t *testing.T, client apiv1connect.OliveTinApiServiceCli resp2, err2 := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{})) require.NoError(t, err2, "Second GetEntities call should not return an error") - assert.Len(t, entityDefinitions, len(resp2.Msg.EntityDefinitions), "Second call should return same number of entity definitions") + require.NotNil(t, resp2.Msg) + require.Len(t, resp2.Msg.EntityDefinitions, len(entityDefinitions), "Second call should return same number of entity definitions") for i, def := range entityDefinitions { assert.Equal(t, def.Title, resp2.Msg.EntityDefinitions[i].Title, "Entity order should be consistent across calls") - assert.Len(t, def.Instances, len(resp2.Msg.EntityDefinitions[i].Instances), "Instance count should be consistent") + require.Len(t, resp2.Msg.EntityDefinitions[i].Instances, len(def.Instances), "Instance count should be consistent") for j, inst := range def.Instances { assert.Equal(t, inst.UniqueKey, resp2.Msg.EntityDefinitions[i].Instances[j].UniqueKey, "Instance order should be consistent across calls") } diff --git a/service/internal/auth/otjwt/jwt_test.go b/service/internal/auth/otjwt/jwt_test.go index ceae685..0d3f690 100644 --- a/service/internal/auth/otjwt/jwt_test.go +++ b/service/internal/auth/otjwt/jwt_test.go @@ -17,6 +17,7 @@ import ( config "github.com/OliveTin/OliveTin/internal/config" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func generateRSAKeyPair(t *testing.T) (*rsa.PrivateKey, []byte) { @@ -116,7 +117,8 @@ func verifyJWTResponse(t *testing.T, res *http.Response, expectCode int) { defer func() { _ = res.Body.Close() }() assert.Equal(t, expectCode, res.StatusCode) - body, _ := io.ReadAll(res.Body) + body, err := io.ReadAll(res.Body) + require.NoError(t, err, "reading JWT response body") t.Logf("Response body: %s", string(body)) } @@ -145,15 +147,9 @@ func testJwkValidationWithAudience(t *testing.T, expire int64, expectCode int, c srv := httptest.NewServer(handler) defer srv.Close() - res := makeJWTRequest(t, srv, tokenStr) + res := makeJWTRequest(t, srv, tokenStr) //nolint:bodyclose // closed by verifyJWTResponse verifyJWTResponse(t, res, expectCode) - - err := res.Body.Close() - - if err != nil { - t.Error("Could not close response body", err) - } } func TestJWTSignatureVerificationSucceeds(t *testing.T) { @@ -238,10 +234,6 @@ func TestJWTHeader(t *testing.T) { srv := httptest.NewServer(mux) defer srv.Close() - res := makeJWTRequest(t, srv, tokenStr) - defer func() { _ = res.Body.Close() }() - - assert.Equal(t, http.StatusOK, res.StatusCode) - body, _ := io.ReadAll(res.Body) - t.Logf("Response body: %s", string(body)) + res := makeJWTRequest(t, srv, tokenStr) //nolint:bodyclose // closed by verifyJWTResponse + verifyJWTResponse(t, res, http.StatusOK) } diff --git a/service/internal/auth/otoauth2/restapi_auth_oauth2.go b/service/internal/auth/otoauth2/restapi_auth_oauth2.go index 9b87f39..afe5c1d 100644 --- a/service/internal/auth/otoauth2/restapi_auth_oauth2.go +++ b/service/internal/auth/otoauth2/restapi_auth_oauth2.go @@ -353,7 +353,7 @@ func getUserInfo(cfg *config.Config, client *http.Client, provider *config.OAuth return ret } - res, err := http.DefaultClient.Do(req) + res, err := client.Do(req) if err != nil { log.Errorf("Failed to get user data: %v", err) diff --git a/service/internal/executor/arguments_test.go b/service/internal/executor/arguments_test.go index 6a77f95..a8c6e2a 100644 --- a/service/internal/executor/arguments_test.go +++ b/service/internal/executor/arguments_test.go @@ -358,7 +358,7 @@ func TestArgumentNotProvided(t *testing.T) { out, err := parseActionArguments(req) assert.Empty(t, out) - assert.Equal(t, "required arg not provided: personName", err.Error()) + require.EqualError(t, err, "required arg not provided: personName") } func TestExecArrayParsing(t *testing.T) { diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index fcd4c18..b7eac12 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -49,8 +49,6 @@ type ActionBinding struct { ConfigOrder int } -// Executor represents a helper class for executing commands. It's main method -// is ExecRequest type Executor struct { logs map[string]*InternalLogEntry LogsByBindingId map[string][]*InternalLogEntry @@ -1282,12 +1280,75 @@ var ( ) func substituteShellAfterCompletedEnvRefs(command string) string { - // $$ is required: regexp replacements treat $ as submatch expansion. - command = shellAfterOutputRef.ReplaceAllString(command, `"$$OUTPUT"`) - command = shellAfterExitCodeRef.ReplaceAllString(command, `"$$EXITCODE"`) + command = replaceShellAfterEnvRef(command, shellAfterOutputRef, "$OUTPUT") + command = replaceShellAfterEnvRef(command, shellAfterExitCodeRef, "$EXITCODE") return command } +func replaceShellAfterEnvRef(command string, pattern *regexp.Regexp, envRef string) string { + matches := pattern.FindAllStringIndex(command, -1) + for i := len(matches) - 1; i >= 0; i-- { + start, end := matches[i][0], matches[i][1] + replacement := `"` + envRef + `"` + if shellPosInsideSingleQuotes(command, start) { + // Break out of single quotes so the env ref can expand at runtime. + replacement = `'` + replacement + `'` + } + command = command[:start] + replacement + command[end:] + } + return command +} + +func shellPosInsideSingleQuotes(command string, pos int) bool { + inSingle := false + inDouble := false + i := 0 + + for i < pos { + inSingle, inDouble, i = advanceShellQuoteState(command, i, pos, inSingle, inDouble) + } + + return inSingle +} + +func advanceShellQuoteState(command string, i, pos int, inSingle, inDouble bool) (bool, bool, int) { + if inSingle { + return advanceInsideSingleQuote(command, i, inSingle, inDouble) + } + if inDouble { + return advanceInsideDoubleQuote(command, i, pos, inSingle, inDouble) + } + return advanceOutsideQuotes(command, i, inSingle, inDouble) +} + +func advanceInsideSingleQuote(command string, i int, inSingle, inDouble bool) (bool, bool, int) { + if command[i] == '\'' { + return false, inDouble, i + 1 + } + return inSingle, inDouble, i + 1 +} + +func advanceInsideDoubleQuote(command string, i, pos int, inSingle, inDouble bool) (bool, bool, int) { + if command[i] == '\\' && i+1 < pos { + return inSingle, inDouble, i + 2 + } + if command[i] == '"' { + return inSingle, false, i + 1 + } + return inSingle, inDouble, i + 1 +} + +func advanceOutsideQuotes(command string, i int, inSingle, inDouble bool) (bool, bool, int) { + switch command[i] { + case '\'': + return true, inDouble, i + 1 + case '"': + return inSingle, true, i + 1 + default: + return inSingle, inDouble, i + 1 + } +} + // shellAfterTemplateArgs omits output/exitCode so templates cannot expand them // raw. Those values are only provided as OUTPUT/EXITCODE process environment. func shellAfterTemplateArgs(args map[string]string) map[string]string { diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index d9c4976..b4a353e 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -449,6 +449,45 @@ func TestShellAfterCompletedUsesOutputEnvSafely(t *testing.T) { assert.True(t, os.IsNotExist(err), "shellAfterCompleted must not execute injected commands from output") } +func TestShellAfterCompletedExpandsQuotedPlaceholders(t *testing.T) { + cases := []struct { + name string + sac string + }{ + {"legacy single-quoted", `printf '%s' '{{ output }}'`}, + {"modern single-quoted", `printf '%s' '{{ .Arguments.output }}'`}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := config.DefaultConfig() + executor := DefaultExecutor(cfg) + mainOutput := "quoted-output-ok" + + action := &config.Action{ + Title: "sac-quoted-" + tc.name, + Shell: "printf %s \"" + mainOutput + "\"", + ShellAfterCompleted: tc.sac, + } + cfg.Actions = append(cfg.Actions, action) + cfg.Sanitize() + executor.RebuildActionMap() + + req := ExecutionRequest{ + AuthenticatedUser: auth.UserFromSystem(cfg, "cron"), + Cfg: cfg, + Binding: executor.FindBindingWithNoEntity(action), + } + wg, _ := executor.ExecRequest(&req) + wg.Wait() + + require.NotNil(t, req.logEntry) + assert.Equal(t, int32(0), req.logEntry.ExitCode) + assert.Contains(t, req.logEntry.Output, "OliveTin::shellAfterCompleted stdout\n"+mainOutput) + }) + } +} + func TestShellAfterCompletedBlocksArgumentsOutputInjection(t *testing.T) { payload := func(injectedPath string) string { return "x; touch " + injectedPath + "; #" @@ -469,25 +508,25 @@ func TestShellAfterCompletedBlocksArgumentsOutputInjection(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { cfg := config.DefaultConfig() - e := DefaultExecutor(cfg) + executor := DefaultExecutor(cfg) injectedPath := filepath.Join(t.TempDir(), "injected") mainPayload := payload(injectedPath) - a1 := &config.Action{ + action := &config.Action{ Title: "sac-injection-" + tc.name, Shell: "printf %s \"" + mainPayload + "\"", ShellAfterCompleted: tc.sac, } - cfg.Actions = append(cfg.Actions, a1) + cfg.Actions = append(cfg.Actions, action) cfg.Sanitize() - e.RebuildActionMap() + executor.RebuildActionMap() req := ExecutionRequest{ AuthenticatedUser: auth.UserFromSystem(cfg, "cron"), Cfg: cfg, - Binding: e.FindBindingWithNoEntity(a1), + Binding: executor.FindBindingWithNoEntity(action), } - wg, _ := e.ExecRequest(&req) + wg, _ := executor.ExecRequest(&req) wg.Wait() _, err := os.Stat(injectedPath) @@ -509,6 +548,10 @@ func TestSubstituteShellAfterCompletedEnvRefs(t *testing.T) { {`echo {{ exitCode }}`, `echo "$EXITCODE"`}, {`echo {{ .Arguments.exitCode }}`, `echo "$EXITCODE"`}, {`echo {{ .Arguments.exitCode }}`, `echo "$EXITCODE"`}, + {`printf '%s' '{{ output }}'`, `printf '%s' ''"$OUTPUT"''`}, + {`printf '%s' '{{ .Arguments.output }}'`, `printf '%s' ''"$OUTPUT"''`}, + {`printf '%s' '{{ exitCode }}'`, `printf '%s' ''"$EXITCODE"''`}, + {`printf '%s' '{{ .Arguments.exitCode }}'`, `printf '%s' ''"$EXITCODE"''`}, } for _, tc := range cases { diff --git a/specs/config-issues.md b/specs/config-issues.md index 2a996c3..631df3d 100644 --- a/specs/config-issues.md +++ b/specs/config-issues.md @@ -8,7 +8,7 @@ Operators should see configuration problems in Diagnostics instead of only in se ## When issues are rebuilt -The issue list is cleared and rebuilt when the action map is rebuilt. That happens after configuration load or reload, and after entity data changes. Some load-time findings that cannot be re-derived after decode (for example unset environment variables already expanded away) are kept across rebuilds until the next configuration load begins. +The issue list is cleared and rebuilt when configuration is loaded or reloaded, and when entity data changes. Some findings that can only be detected while configuration is first being loaded (for example references to unset environment variables that are expanded away during that load) are kept across later rebuilds until the next configuration load begins. ## What is collected @@ -16,7 +16,7 @@ Issues include: - Unknown or unenforced action group references - Checklist arguments with missing or invalid choice templates -- Arguments whose type was left unset (defaulted to ascii) +- Arguments whose type was left unset (defaulted to a generic text type) - Unset environment variables referenced from configuration - Missing or invalid include directories - Argument default or choice templates that fail to parse @@ -29,7 +29,7 @@ Issues include: Each issue has a severity of warning or error, a stable code, a human-readable message, and optional context such as action title, argument name, configuration source file, or detail value. -When the issue list is rebuilt, OliveTin logs only newly appeared issues so startup does not repeat the same warning for every action-map rebuild. +When the issue list is rebuilt, OliveTin logs only newly appeared issues so startup does not repeat the same warning for every rebuild. When configuration is loaded from a base file and an include directory, OliveTin records which file defined each action and entity declaration. That path is shown as the configuration source file when available. Some issues (for example unset environment variables) may not have a specific file. Entity data file problems also show the entity data path in the detail column. @@ -45,6 +45,6 @@ Users who are not allowed to view Diagnostics cannot retrieve the issue list. When Diagnostics is visible and at least one configuration issue exists that the user is allowed to see, the Diagnostics navigation link shows a count badge with the number of those issues. The badge clears when the visible issue count becomes zero after a configuration or entity refresh. -## Init count +## Startup count -The Init response includes the configuration issue count for users who may view Diagnostics, using the same per-user filtering as the Diagnostics list. For other users the count is zero. +When the web UI starts, users who may view Diagnostics receive the same filtered configuration issue count used for the Diagnostics list and navigation badge. For other users the count is zero. diff --git a/specs/dashboard-nav-categories.md b/specs/dashboard-nav-categories.md index 671c615..1ad61be 100644 --- a/specs/dashboard-nav-categories.md +++ b/specs/dashboard-nav-categories.md @@ -28,7 +28,7 @@ When building the sidebar: 3. Within a category, dashboards keep the order they appear in the configuration among visible dashboards in that category. 4. After all dashboard links, a **System** category lists Entities, Logs, and Diagnostics (each only when the user is allowed to see that item). If none of those links are visible, the System category is omitted. -The default Actions dashboard, when present, is uncategorized unless configuration gives it a category (it is not a configured root entry, so it stays uncategorized). +The default Actions dashboard, when present, is always uncategorized. ## 4. Navigation style