From 5295c932c09d0f372aceb493d19d842bc10c03ee Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 08:12:54 +0000 Subject: [PATCH] fix(addons): check the private egress phrase on the owner With add-ons enabled /api/settings rejects chats_egress_allow_private, so the typed phrase check there never ran on the only write path, the host-settings-update action. The page asked for the phrase but dropped it from the patch, and a direct admin call could turn the switch on without it. The patch now carries chats_egress_private_confirm and hostsettings.apply rejects turning the switch on without it. The phrase is hashed into the operation receipt like the rest of the patch and is never stored. The e2e fixture enforces the same rule and the egress spec asserts the phrase in the owner patch again. Assisted-by: Claude Code:claude-sonnet-5-5 Signed-off-by: Ettore Di Giacinto --- core/http/react-ui/e2e/addons-fixtures.js | 6 +++++ core/http/react-ui/e2e/chats-egress.spec.js | 7 ++--- .../hostcontrols/HostControlsForm.jsx | 7 +++-- .../addons/hostsettings/authority_test.go | 27 +++++++++++++++++++ core/services/addons/hostsettings/policy.go | 11 ++++++++ 5 files changed, 51 insertions(+), 7 deletions(-) diff --git a/core/http/react-ui/e2e/addons-fixtures.js b/core/http/react-ui/e2e/addons-fixtures.js index ab561aaa94..595949b933 100644 --- a/core/http/react-ui/e2e/addons-fixtures.js +++ b/core/http/react-ui/e2e/addons-fixtures.js @@ -86,6 +86,7 @@ export function hostPolicy(over = {}) { function applyHostPatch(policy, patch) { const next = structuredClone(policy) for (const [key, value] of Object.entries(patch || {})) { + if (key === 'chats_egress_private_confirm') continue // checked by the owner, never stored if (key !== 'chats_tool_providers') { next[key] = value; continue } for (const [name, provider] of Object.entries(value || {})) { if (provider === null) delete next.chats_tool_providers[name] @@ -258,6 +259,11 @@ export async function mockAddons(page, options = {}) { return route.fulfill(ok({ data: { schema_version: 1, revision: state.hostSettings.revision, policy: state.hostSettings.policy } })) case 'host-settings-update': { if (args.expectedRevision !== state.hostSettings.revision) return route.fulfill(ok({ error: 'revision_conflict' })) + // Like hostsettings.apply: turning the private switch on needs the phrase. + const turnsPrivateOn = args.patch?.chats_egress_allow_private === true && !state.hostSettings.policy.chats_egress_allow_private + if (turnsPrivateOn && String(args.patch.chats_egress_private_confirm || '').trim() !== 'I understand that commands can reach my private network') { + return route.fulfill(ok({ error: 'invalid' })) + } state.log.hostSettings.push(args.patch) state.hostSettings = { revision: state.hostSettings.revision + 1, policy: applyHostPatch(state.hostSettings.policy, args.patch) } return route.fulfill(ok({ data: { schema_version: 1, revision: state.hostSettings.revision, policy: state.hostSettings.policy } })) diff --git a/core/http/react-ui/e2e/chats-egress.spec.js b/core/http/react-ui/e2e/chats-egress.spec.js index 79ec1c15f1..66762a05e6 100644 --- a/core/http/react-ui/e2e/chats-egress.spec.js +++ b/core/http/react-ui/e2e/chats-egress.spec.js @@ -371,12 +371,13 @@ test.describe('The network of the commands on the add-on page', () => { expect(settings.log.dispatch.filter(r => r.action === 'host-settings-update')).toHaveLength(0) await page.getByTestId('settings-egress-private-phrase').fill('I understand that commands can reach my private network') await page.getByRole('button', { name: 'Apply' }).click() - // The phrase is checked in the page; the owner patch carries policy fields only. - await expect.poll(() => settings.log.hostSettings).toEqual([{ chats_egress_allow_private: true }]) + // The owner checks the phrase too, so the patch carries it. + const phrase = 'I understand that commands can reach my private network' + await expect.poll(() => settings.log.hostSettings).toEqual([{ chats_egress_allow_private: true, chats_egress_private_confirm: phrase }]) const writes = settings.log.dispatch.filter(r => r.action === 'host-settings-update') expect(writes).toHaveLength(1) expect(writes[0].operationId).toBeTruthy() - expect(writes[0].arguments).toEqual({ expectedRevision: 1, patch: { chats_egress_allow_private: true } }) + expect(writes[0].arguments).toEqual({ expectedRevision: 1, patch: { chats_egress_allow_private: true, chats_egress_private_confirm: phrase } }) await expect(page.getByTestId('settings-pending')).toHaveCount(0) }) diff --git a/core/http/react-ui/src/components/hostcontrols/HostControlsForm.jsx b/core/http/react-ui/src/components/hostcontrols/HostControlsForm.jsx index 4e8cb83b8d..39a6ed4df4 100644 --- a/core/http/react-ui/src/components/hostcontrols/HostControlsForm.jsx +++ b/core/http/react-ui/src/components/hostcontrols/HostControlsForm.jsx @@ -3,7 +3,7 @@ import { Suspense, useMemo, useState } from 'react' import { Callout, LoadError, SkeletonRows } from '../ui' import PendingBar from '../settings/PendingBar' import UnsavedChangesGuard from '../UnsavedChangesGuard' -import { EGRESS_CONFIRM_KEY, pendingChanges, payloadFor, runChecks, visibleChanges } from '../../utils/settingsPending' +import { pendingChanges, payloadFor, runChecks, visibleChanges } from '../../utils/settingsPending' import { createHostSettingsClient } from '../../utils/hostSettingsClient' import { createAddonsApi } from '../../utils/addonsApi' import { apiUrl } from '../../utils/basePath' @@ -52,9 +52,8 @@ export default function HostControlsForm({ app, client = ownerClient }) { saving={saving} showDiff={showDiff} onToggleDiff={() => setShowDiff(v => !v)} onApply={() => { const patch = payloadFor(changes, settings) - // The typed phrase is checked above, but only the legacy runtime - // endpoint accepts it. The owner patch contains policy fields only. - delete patch[EGRESS_CONFIRM_KEY] + // The phrase stays in the patch: the owner checks it too, so a + // direct admin API call cannot skip the confirmation. if (patch.chats_tool_providers) { const before = initial.chats_tool_providers, after = patch.chats_tool_providers patch.chats_tool_providers = Object.fromEntries([...new Set([...Object.keys(before), ...Object.keys(after)])].filter(k => before[k] !== after[k]).map(k => [k, after[k] ?? null])) diff --git a/core/services/addons/hostsettings/authority_test.go b/core/services/addons/hostsettings/authority_test.go index b736a8b251..759dad9f8b 100644 --- a/core/services/addons/hostsettings/authority_test.go +++ b/core/services/addons/hostsettings/authority_test.go @@ -12,6 +12,8 @@ import ( "strings" "testing" + "github.com/mudler/LocalAI/pkg/egresspolicy" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -75,6 +77,31 @@ var _ = Describe("Owner policy", func() { Expect(err).NotTo(HaveOccurred()) Expect(info.Mode().Perm()).To(Equal(os.FileMode(0600))) }) + It("turns private egress on only with the typed phrase, and never stores it", func() { + open() + _, err := s.Update(ctx, Patch{AllowPrivate: ptr(true)}, 1, "op-no-phrase") + Expect(err).To(MatchError(ErrInvalid)) + _, err = s.Update(ctx, Patch{AllowPrivate: ptr(true), EgressPrivateConfirm: ptr("yes")}, 1, "op-wrong-phrase") + Expect(err).To(MatchError(ErrInvalid)) + live, err := s.Snapshot(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(live.Revision).To(Equal(uint64(1))) + Expect(live.Policy.AllowPrivate).To(BeFalse()) + + got, err := s.Update(ctx, Patch{AllowPrivate: ptr(true), EgressPrivateConfirm: ptr(" " + egresspolicy.EgressPrivatePhrase + " ")}, 1, "op-phrase") + Expect(err).NotTo(HaveOccurred()) + Expect(got.Policy.AllowPrivate).To(BeTrue()) + raw, err := os.ReadFile(path) + Expect(err).NotTo(HaveOccurred()) + Expect(string(raw)).NotTo(ContainSubstring(egresspolicy.EgressPrivatePhrase)) + + // Already on: later patches need no phrase, and switching off never does. + _, err = s.Update(ctx, Patch{Ports: ptr("8443")}, 2, "op-ports") + Expect(err).NotTo(HaveOccurred()) + off, err := s.Update(ctx, Patch{AllowPrivate: ptr(false)}, 3, "op-off") + Expect(err).NotTo(HaveOccurred()) + Expect(off.Policy.AllowPrivate).To(BeFalse()) + }) It("prunes oldest receipts and preserves restart replay without limiting updates", func() { open() // Seed the full retention window through real reducer transitions, then diff --git a/core/services/addons/hostsettings/policy.go b/core/services/addons/hostsettings/policy.go index 8f0c7f7450..de0535b5e2 100644 --- a/core/services/addons/hostsettings/policy.go +++ b/core/services/addons/hostsettings/policy.go @@ -67,6 +67,10 @@ type Patch struct { AllowedHosts *string `json:"chats_egress_allowed_hosts,omitempty"` ToolProviders map[string]*string `json:"chats_tool_providers,omitempty"` AutoUpgradeAddons *bool `json:"auto_upgrade_addons,omitempty"` + // EgressPrivateConfirm carries the typed phrase that turning on + // chats_egress_allow_private requires. The owner checks it, so a direct + // call cannot skip what the page asks for. It is never stored. + EgressPrivateConfirm *string `json:"chats_egress_private_confirm,omitempty"` } func (p *Patch) UnmarshalJSON(b []byte) error { @@ -258,6 +262,10 @@ func validate(p Policy) error { return nil } +func confirmedPrivate(phrase *string) bool { + return phrase != nil && strings.TrimSpace(*phrase) == egresspolicy.EgressPrivatePhrase +} + func apply(s Snapshot, p Patch) (Snapshot, error) { s = clone(s) for _, f := range []struct { @@ -269,6 +277,9 @@ func apply(s Snapshot, p Patch) (Snapshot, error) { } } if p.AllowPrivate != nil { + if *p.AllowPrivate && !s.Policy.AllowPrivate && !confirmedPrivate(p.EgressPrivateConfirm) { + return Snapshot{}, fmt.Errorf("%w: chats_egress_allow_private needs chats_egress_private_confirm", ErrInvalid) + } s.Policy.AllowPrivate = *p.AllowPrivate } if p.AutoUpgradeAddons != nil {