From e8e2d1ff7c3f85f6e8b60599bbf9310315391894 Mon Sep 17 00:00:00 2001 From: jamesread Date: Tue, 7 Jul 2026 13:02:20 +0100 Subject: [PATCH 1/2] chore: add SECURITY_ADVISORY_DUPLICATES maintainer guide Document known duplicate advisory clusters and triage steps referenced from SECURITY.md. Co-authored-by: Cursor --- .github/SECURITY_ADVISORY_DUPLICATES.md | 116 ++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 .github/SECURITY_ADVISORY_DUPLICATES.md diff --git a/.github/SECURITY_ADVISORY_DUPLICATES.md b/.github/SECURITY_ADVISORY_DUPLICATES.md new file mode 100644 index 0000000..22f59fc --- /dev/null +++ b/.github/SECURITY_ADVISORY_DUPLICATES.md @@ -0,0 +1,116 @@ +# Security Advisory Duplicates — Maintainer Guide + +This document lists known duplicate security advisory clusters for [OliveTin/OliveTin](https://github.com/OliveTin/OliveTin). When triaging new reports, check here and [open advisories](https://github.com/OliveTin/OliveTin/security/advisories) before accepting. + +**Duplicate policy:** the earliest reporter on the canonical advisory receives primary credit. Later reporters are credited on the canonical advisory when closed as duplicates. See [SECURITY.md](../SECURITY.md). + +## Triage checklist + +1. Search open advisories for the same component and attack path. +2. Match against clusters below. +3. If duplicate: close the newer advisory, link to canonical, add reporter to canonical credits. +4. If unique: accept, patch on a private branch, reassess CVSS with OliveTin context (see SECURITY.md — OliveTin is intentional RCE by design). +5. Merge fix to `next`, publish advisory, credit reporters in advisory body (not commit message). + +--- + +## shellAfterCompleted command injection + +**Canonical:** [GHSA-vc6p-m6vx-6cwq](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-vc6p-m6vx-6cwq) — reporter **knight-yagami** (2026-03-04) + +Untrusted command output or template variables interpolated into `shellAfterCompleted` and executed via `sh -c`. + +| GHSA | Reporter | Status | Notes | +|------|----------|--------|-------| +| GHSA-vc6p-m6vx-6cwq | knight-yagami | closed (canonical) | Original report | +| GHSA-v5gc-hqpq-227p | 0xkakash1 | closed (duplicate) | Output template variant | +| GHSA-m7wr-wj5j-7459 | Ryu7zz | duplicate | Webhook `exec` → output → `shellAfterCompleted` | +| GHSA-cjxm-x848-6vmc | Yesuhei | duplicate | Missing shell safety on after-completion | +| GHSA-j9p9-36jc-2v8w | anushkavirgaonkar | duplicate | Same root cause | + +**Fix:** shell-quote `output`/`exitCode` before template render; block `shellAfterCompleted` for webhook-tagged actions. + +**CVSS note:** requires admin-configured `shellAfterCompleted` and attacker influence on output — typically PR:H not PR:N. + +--- + +## OAuth2 state map memory exhaustion (DoS) + +**Canonical:** [GHSA-xpxj-f2fm-rqch](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xpxj-f2fm-rqch) — reporter **knight-yagami** (2026-03-04) + +Unauthenticated `/oauth/login` grows `registeredStates` without TTL or cap. + +| GHSA | Reporter | Status | Notes | +|------|----------|--------|-------| +| GHSA-xpxj-f2fm-rqch | knight-yagami | closed (canonical) | Original report | +| GHSA-cj96-c55v-2f3c | Dredsen | duplicate | Same unbounded map | + +**Fix:** TTL sweep (match 15-minute cookie MaxAge), max map size, cleanup on failed callback. + +**CVSS note:** unauthenticated DoS — reported 7.5 is appropriate. + +--- + +## URL argument type — unrestricted URI schemes (SSRF / file read) + +**Canonical:** [GHSA-45pc-w4ph-hrq4](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-45pc-w4ph-hrq4) — reporter **fg0x0** (2026-03-09) + +`url` type accepts `file://`, `gopher://`, etc. Blocked in `shell:` mode but still validated weakly for `exec:` actions. + +| GHSA | Reporter | Status | Notes | +|------|----------|--------|-------| +| GHSA-45pc-w4ph-hrq4 | fg0x0 | closed (canonical) | Original report | +| GHSA-cchg-25m4-q6rj | anushkavirgaonkar | duplicate | Same scheme validation gap | + +**Fix:** allowlist `http`/`https` in `typeSafetyCheckUrl`. + +**CVSS note:** admin must configure `exec:` action passing URL to external tool — PR:H. + +--- + +## Custom `regex:` argument type in shell actions + +**Canonical:** [GHSA-xc5w-4v5w-7x65](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xc5w-4v5w-7x65) — reporter **Ayantaker** (2026-05-06) + +`regex:` types not in shell denylist; partial `MatchString` allows injection suffixes. + +| GHSA | Reporter | Status | Notes | +|------|----------|--------|-------| +| GHSA-xc5w-4v5w-7x65 | Ayantaker | canonical | Missing denylist entry | +| GHSA-gvxq-7gvp-4ggr | anushkavirgaonkar | duplicate | Unanchored partial match | + +**Fix:** deny `regex:` in shell mode; enforce full-string match for custom regex types. + +--- + +## Shell denylist incomplete (post CVE-2026-27626) + +**Canonical:** [GHSA-c26w-h42g-jfp9](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-c26w-h42g-jfp9) — reporter **sec-reex** (2026-07-03) + +CVE-2026-27626 added `password` to denylist only; `html`, `confirmation`, and choiceless `checkbox` still skip validation and are allowed in `shell:` actions. + +**Fix:** extend `checkShellArgumentSafety` denylist. + +--- + +## StartActionAndWait logs ACL bypass + +**Canonical:** [GHSA-jm28-2wcr-qf3h](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-jm28-2wcr-qf3h) — reporter **offset** (2026-03-12) + +`StartActionAndWait` / `StartActionByGetAndWait` return full log output without `logs` ACL check. + +No known duplicates. + +**Fix:** apply `isLogEntryAllowed` before returning `LogEntry`. + +**CVSS note:** requires authenticated user with `exec` but not `logs` — typically 4.3–5.3 not 6.5. + +--- + +## Easy to confuse (not duplicates) + +| Topic | Advisories | Distinction | +|-------|------------|-------------| +| OAuth2 state DoS vs OAuth2 auth bypass | GHSA-xpxj vs GHSA-3v7p | DoS fills state map; bypass spoofs `authHttpHeaderUsername` | +| `shellAfterCompleted` vs direct `shell` injection | GHSA-vc6p vs GHSA-49gm | Second-order via output vs first-order argument injection | +| `ValidateArgumentType` enumeration | GHSA-f637 vs GHSA-x6q3 | Same issue; GHSA-f637 published | From 904616b1b1ef995ab868431b0835cb3fc368be7b Mon Sep 17 00:00:00 2001 From: jamesread Date: Wed, 8 Jul 2026 09:52:30 +0100 Subject: [PATCH 2/2] security: GHSA-jm28-2wcr-qf3h (LOW) enforce logs ACL on sync execution endpoints StartActionAndWait and StartActionByGetAndWait now apply the same logs permission check used by GetLogs and ExecutionStatus. Co-authored-by: Cursor --- service/internal/api/api.go | 70 +++++++++++++++++++++++--------- service/internal/api/api_test.go | 48 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/service/internal/api/api.go b/service/internal/api/api.go index 77cd945..ed546f6 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -282,6 +282,21 @@ func (api *oliveTinAPI) findBindingByIDOrNotFound(bindingId string) (*executor.A return api.findBindingOrNotFound(bindingId) } +func (api *oliveTinAPI) startActionAndWaitLogEntry(binding *executor.ActionBinding, args map[string]string, justification string, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) { + internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user) + if !ok { + return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found")) + } + return api.logEntryForAllowedViewer(internalLogEntry, user) +} + +func (api *oliveTinAPI) logEntryForAllowedViewer(internalLogEntry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) { + if err := api.requireLogEntryAllowed(internalLogEntry, user); err != nil { + return nil, err + } + return api.internalLogEntryToPb(internalLogEntry, user), nil +} + func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionAndWaitRequest]) (*connect.Response[apiv1.StartActionAndWaitResponse], error) { binding, err := api.findBindingOrNotFound(req.Msg.ActionId) if err != nil { @@ -295,12 +310,12 @@ func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request return nil, connectInvalidJustification(err) } - internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user) - if !ok { - return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found")) + logEntry, err := api.startActionAndWaitLogEntry(binding, args, justification, user) + if err != nil { + return nil, err } return connect.NewResponse(&apiv1.StartActionAndWaitResponse{ - LogEntry: api.internalLogEntryToPb(internalLogEntry, user), + LogEntry: logEntry, }), nil } @@ -327,16 +342,7 @@ func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *connect.Request[a }), nil } -func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) { - binding := api.executor.FindBindingByID(req.Msg.ActionId) - if binding == nil || binding.Action == nil { - return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId)) - } - - args := make(map[string]string) - - user := auth.UserFromApiCall(ctx, req, api.cfg) - +func (api *oliveTinAPI) runBindingAndWait(binding *executor.ActionBinding, args map[string]string, user *authpublic.AuthenticatedUser) (*executor.InternalLogEntry, bool) { execReq := executor.ExecutionRequest{ Binding: binding, TrackingID: uuid.NewString(), @@ -348,14 +354,31 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Re wg, _ := api.executor.ExecRequest(&execReq) wg.Wait() - internalLogEntry, ok := api.executor.GetLog(execReq.TrackingID) + return api.executor.GetLog(execReq.TrackingID) +} - if ok { - return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{ - LogEntry: api.internalLogEntryToPb(internalLogEntry, user), - }), nil +func (api *oliveTinAPI) startActionByGetAndWaitLogEntry(binding *executor.ActionBinding, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) { + internalLogEntry, ok := api.runBindingAndWait(binding, map[string]string{}, user) + if !ok { + return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found")) } - return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found")) + return api.logEntryForAllowedViewer(internalLogEntry, user) +} + +func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) { + binding := api.executor.FindBindingByID(req.Msg.ActionId) + if binding == nil || binding.Action == nil { + return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId)) + } + + user := auth.UserFromApiCall(ctx, req, api.cfg) + logEntry, err := api.startActionByGetAndWaitLogEntry(binding, user) + if err != nil { + return nil, err + } + return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{ + LogEntry: logEntry, + }), nil } func calculateRateLimitExpires(api *oliveTinAPI, logEntry *executor.InternalLogEntry) string { @@ -684,6 +707,13 @@ func (api *oliveTinAPI) isLogEntryAllowed(e *executor.InternalLogEntry, user *au return acl.IsAllowedLogs(api.cfg, user, e.Binding.Action) } +func (api *oliveTinAPI) requireLogEntryAllowed(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) error { + if api.isLogEntryAllowed(entry, user) { + return nil + } + return connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied to view this execution")) +} + // mayViewExecutionEvent returns whether the user is allowed to receive this execution event (for EventStream ACL). func (api *oliveTinAPI) mayViewExecutionEvent(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) bool { if user == nil { diff --git a/service/internal/api/api_test.go b/service/internal/api/api_test.go index d2ed297..e00394c 100644 --- a/service/internal/api/api_test.go +++ b/service/internal/api/api_test.go @@ -385,6 +385,54 @@ func testWithEntity(t *testing.T, binding *executor.ActionBinding, rr *Dashboard assert.Equal(t, expectedCanExec, actionResult.CanExec, message) } +// buildExecWithoutLogsTestConfig returns config for GHSA-jm28: user "runner" may exec but not read logs. +func buildExecWithoutLogsTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser) { + t.Helper() + cfg := config.DefaultConfig() + cfg.AuthHttpHeaderUsername = "X-Ot-User" + cfg.DefaultPermissions.View = false + cfg.DefaultPermissions.Exec = false + cfg.DefaultPermissions.Logs = false + + cfg.Actions = append(cfg.Actions, &config.Action{ + ID: "run_only", + Title: "Run Only", + Shell: "echo sensitive-output", + Icon: "🔒", + }) + + cfg.AccessControlLists = append(cfg.AccessControlLists, &config.AccessControlList{ + Name: "runner", + MatchUsernames: []string{"runner"}, + AddToEveryAction: true, + Permissions: config.PermissionsList{View: true, Exec: true, Logs: false, Kill: false}, + }) + + runner := &authpublic.AuthenticatedUser{Username: "runner"} + runner.BuildUserAcls(cfg) + return cfg, runner +} + +// TestStartActionAndWaitDeniesLogsPermission (GHSA-jm28-2wcr-qf3h) asserts sync execution endpoints +// enforce logs ACL and do not return action output to users allowed to exec but not read logs. +func TestStartActionAndWaitDeniesLogsPermission(t *testing.T) { + cfg, _ := buildExecWithoutLogsTestConfig(t) + ex := executor.DefaultExecutor(cfg) + ex.RebuildActionMap() + ts, client := getNewTestServerAndClientWithExecutor(cfg, ex) + defer ts.Close() + + req := connect.NewRequest(&apiv1.StartActionAndWaitRequest{ + ActionId: "run_only", + }) + req.Header().Set("X-Ot-User", "runner") + + _, err := client.StartActionAndWait(context.Background(), req) + require.Error(t, err) + assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err), + "user with exec:true and logs:false must not receive log output from StartActionAndWait") +} + // buildViewPermissionTestConfig returns config and users for GHSA view-permission tests: // one action "secret_action", ACL "restricted" (view:false, logs:false) for user "low", ACL "full" (view:true, logs:true) for user "admin". func buildViewPermissionTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser, *authpublic.AuthenticatedUser) {