From 2b588a7350fa25c493e5a968b141e2c5b3c17ac4 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Tue, 3 Nov 2026 15:41:32 +0000 Subject: [PATCH] fix(connectors): hold add-on write tools to tools.write in connector chats A write tool of an add-on that holds a credential was protected only by the write mode of the connection and the confirmation of the proposal. In a connector chat it now also needs the capability tools.write for the sender of the turn, through the same Authorize path as the other capabilities: the tool is not offered without it, a call is checked again and every decision is audited. The default is no. The rule is by shape, not by name. The manifest says an add-on holds a credential, and contract.ToolPermission maps its non-read tools to tools.write. The chat reads the permission from the tool definition, so the core names no add-on. Also close a hole in local-owner mode, where a chat turn and the browser share one credential: a proposal that waits for a person could be confirmed from a chat turn. The call of a chat tool now carries a mark, and the owner refuses to confirm such a proposal under it. A remote owner does not see the mark; local-owner mode has no remote owner. Assisted-by: Claude:claude-sonnet-5-5 go test --- core/application/addon_gateway.go | 2 +- core/services/addons/apps/apps.go | 5 +++ core/services/addons/gateway/chatturn.go | 21 ++++++++++ core/services/addons/policy/proposals.go | 5 ++- .../policy/proposals_credential_test.go | 15 +++++++ core/services/addons/tools/adapter.go | 10 ++++- .../services/addons/tools/definitions_test.go | 26 +++++++++++++ core/services/studio/addontools.go | 39 ++++++++++++++++++- docs/content/features/chats-connectors.md | 11 ++++++ pkg/addons/contract/connector_test.go | 18 +++++++++ pkg/addons/contract/permissions.go | 13 +++++++ 11 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 core/services/addons/gateway/chatturn.go diff --git a/core/application/addon_gateway.go b/core/application/addon_gateway.go index f7b81e1a06..373449f28d 100644 --- a/core/application/addon_gateway.go +++ b/core/application/addon_gateway.go @@ -805,7 +805,7 @@ func (s *studioAddons) Tools(ctx context.Context, user string) ([]studio.AddonTo continue } out = append(out, studio.AddonTool{Name: d.Name, AddonID: d.AddonID, Description: d.Description, Schema: raw, Action: d.Kind != contract.ToolKindRead, - Shadows: d.Shadows, Execution: d.Execution, InstalledAt: d.InstalledAt}) + Shadows: d.Shadows, Execution: d.Execution, InstalledAt: d.InstalledAt, Permission: d.Permission}) } return filterByFeature(out, s.hostOps, s.has, actor), nil } diff --git a/core/services/addons/apps/apps.go b/core/services/addons/apps/apps.go index d3a1b80a37..b307576a6b 100644 --- a/core/services/addons/apps/apps.go +++ b/core/services/addons/apps/apps.go @@ -116,6 +116,10 @@ type App struct { // the argument schema and the kind of each tool. The host serves them to // chat from the verified manifest; the add-on process is never asked. ToolDefinitions []contract.ToolDefinition `json:"toolDefinitions,omitempty"` + // Credentialed is true when the manifest declares credentials: the tools + // work with a secret that the person keeps in a connector binding. A + // connector chat holds its write tools to contract.ToolPermission. + Credentialed bool `json:"credentialed,omitempty"` // Resources is the resource mapping of the manifest, with the listing // declaration the host acts on. It is not part of the apps API. Resources *contract.ResourceTools `json:"-"` @@ -267,6 +271,7 @@ func build(s Snapshot, admin bool) App { d.InputSchema = slices.Clone(d.InputSchema) a.ToolDefinitions = append(a.ToolDefinitions, d) } + a.Credentialed = len(m.Capabilities.Credentials) > 0 a.Resources = resourceTools(m) a.Skills = slices.Clone(m.Capabilities.SkillDefinitions) a.SkillShadows = contract.ShadowedSkills(m.Capabilities.SkillDefinitions) diff --git a/core/services/addons/gateway/chatturn.go b/core/services/addons/gateway/chatturn.go new file mode 100644 index 0000000000..7393ccac04 --- /dev/null +++ b/core/services/addons/gateway/chatturn.go @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +package gateway + +import "context" + +type chatTurnKey struct{} + +// WithChatTurn marks a context as the call of a tool in a chat turn. In +// local-owner mode a chat turn and the browser share one credential, so the +// credential alone cannot tell them apart; this mark, set by the code that +// runs a model's tool call and read where a proposal is confirmed, does. A +// proposal that waits for a person is never confirmed under it. +func WithChatTurn(ctx context.Context) context.Context { + return context.WithValue(ctx, chatTurnKey{}, true) +} + +// IsChatTurn reports whether ctx belongs to the call of a tool in a chat turn. +func IsChatTurn(ctx context.Context) bool { + v, _ := ctx.Value(chatTurnKey{}).(bool) + return v +} diff --git a/core/services/addons/policy/proposals.go b/core/services/addons/policy/proposals.go index da538245b3..6560fba922 100644 --- a/core/services/addons/policy/proposals.go +++ b/core/services/addons/policy/proposals.go @@ -1528,7 +1528,10 @@ func (b *Broker) execute(ctx context.Context, actor contract.Actor, id string) ( if grant.ResourceID != r.ResourceID { return contract.Receipt{}, ErrForbidden } - if (r.Await || grant.WriteMode != gateway.WriteAuto) && !b.confirmPerson(b.policy, actor) { + // A proposal that waits for a person is confirmed at a host surface, + // never from a chat turn: in local-owner mode the credential of a + // chat turn is the browser's, so the mark of the call decides. + if (r.Await || grant.WriteMode != gateway.WriteAuto) && (gateway.IsChatTurn(ctx) || !b.confirmPerson(b.policy, actor)) { return contract.Receipt{}, ErrForbidden } } diff --git a/core/services/addons/policy/proposals_credential_test.go b/core/services/addons/policy/proposals_credential_test.go index df68039b22..76de6a1ae6 100644 --- a/core/services/addons/policy/proposals_credential_test.go +++ b/core/services/addons/policy/proposals_credential_test.go @@ -142,6 +142,21 @@ var _ = Describe("the write tools of an add-on with a credential", Label("propos Expect(spy.applies[0].Request.Credential).To(Equal(secret)) Expect(spy.applies[0].Request.ResourceKey).To(Equal("acme/widgets")) }) + It("does not let a chat turn confirm it when the chat and the browser share one credential", func() { + // Local-owner mode: one credential stands for both, so only the mark + // of the call tells a chat turn from the browser. + setup("") + f.broker.opts.ConfirmCredential = func(contract.Actor) bool { return true } + f.broker.opts.ConfirmPerson = func(contract.Actor) bool { return true } + _, err := f.broker.Propose(ctx, who, request("op-1", "acme/widgets")) + Expect(err).NotTo(HaveOccurred()) + _, err = f.broker.Confirm(gateway.WithChatTurn(ctx), who, "op-1") + Expect(err).To(MatchError(ErrForbidden)) + Expect(spy.applies).To(BeEmpty()) + receipt, err := f.broker.Confirm(ctx, who, "op-1") + Expect(err).NotTo(HaveOccurred(), "the same proposal is still pending for the browser") + Expect(receipt.State).To(Equal(contract.ReceiptCommitted)) + }) It("refuses a repository that is not on the list before the add-on is called", func() { _, err := f.broker.Propose(ctx, who, request("op-1", "acme/secret")) var refusal *gateway.CredentialRefusal diff --git a/core/services/addons/tools/adapter.go b/core/services/addons/tools/adapter.go index 1ac821652d..3b0bcfbb07 100644 --- a/core/services/addons/tools/adapter.go +++ b/core/services/addons/tools/adapter.go @@ -120,6 +120,10 @@ type ToolDefinition struct { // InstalledAt is when the add-on was first installed (RFC 3339, UTC), or // empty. The chat consumer ranks the add-ons that shadow one name by it. InstalledAt string `json:"installedAt,omitempty"` + // Permission is the capability (see contract.ToolPermission) that a + // sender needs to use the tool in a connector chat, or empty. The chat + // consumer holds the tool to it; another consumer has no sender. + Permission string `json:"permission,omitempty"` } // ResultKind says what a ToolResult carries. @@ -606,7 +610,7 @@ func definition(a apps.App, d contract.ToolDefinition) (ToolDefinition, bool) { if json.Unmarshal(d.InputSchema, ¶ms) != nil { return ToolDefinition{}, false } - def := ToolDefinition{Name: name, AddonID: a.ID, Tool: d.Name, Kind: d.Kind, Description: d.Description, Parameters: params, InstalledAt: a.InstalledAt} + def := ToolDefinition{Name: name, AddonID: a.ID, Tool: d.Name, Kind: d.Kind, Description: d.Description, Parameters: params, InstalledAt: a.InstalledAt, Permission: contract.ToolPermission(a.Credentialed, d)} // Checked again for the same reason as the schema: a manifest that names a // tool outside the list, or a shadow of the wrong kind or schema, never // shadows it. @@ -823,7 +827,9 @@ func (t *ToolAdapter) execute(ctx context.Context, actor contract.Actor, convers // failure that is certain withdraws the proposal, so nothing waits for a // browser; an unknown outcome is left alone and reported, never repeated. func (t *ToolAdapter) applyNow(ctx context.Context, actor contract.Actor, name string, p gateway.Proposal) (ToolResult, error) { - done, err := t.apply.ConfirmProposal(ctx, actor, p.ID, p.Digest) + // The mark keeps the owner from treating this confirmation as a person's + // when the chat and the browser share one credential (local-owner mode). + done, err := t.apply.ConfirmProposal(gateway.WithChatTurn(ctx), actor, p.ID, p.Digest) if err != nil { var unknown *gateway.OutcomeUnknown var unavailable *gateway.ProposalUnavailable diff --git a/core/services/addons/tools/definitions_test.go b/core/services/addons/tools/definitions_test.go index b14606f7e5..079bcf34b4 100644 --- a/core/services/addons/tools/definitions_test.go +++ b/core/services/addons/tools/definitions_test.go @@ -273,3 +273,29 @@ var _ = Describe("typed execution status model exclusion", func() { Expect(f.calls()).To(BeZero()) }) }) + +var _ = Describe("The permission a tool declares to the chat", func() { + ctx := context.Background() + discover := func(credentialed bool) map[string]string { + app := tablesApp(tablesManifest()) + app.Credentialed = credentialed + defs, err := tools.NewChatToolAdapter(&fake{list: apps.List{Apps: []apps.App{app}}}).Discover(ctx, member) + Expect(err).NotTo(HaveOccurred()) + out := map[string]string{} + for _, d := range defs { + out[d.Tool] = d.Permission + } + return out + } + It("is tools.write for the tools that change things, in an add-on that holds a credential", func() { + got := discover(true) + Expect(got["databases-inspect"]).To(BeEmpty(), "a read needs nothing") + Expect(got["tables-create"]).To(Equal(contract.CapToolsWrite)) + Expect(got["databases-create"]).To(Equal(contract.CapToolsWrite)) + }) + It("is empty for every tool of an add-on without a credential", func() { + for name, p := range discover(false) { + Expect(p).To(BeEmpty(), name) + } + }) +}) diff --git a/core/services/studio/addontools.go b/core/services/studio/addontools.go index 92dcc56992..262d5e9999 100644 --- a/core/services/studio/addontools.go +++ b/core/services/studio/addontools.go @@ -10,6 +10,7 @@ import ( "sort" addontools "github.com/mudler/LocalAI/core/services/addons/tools" + "github.com/mudler/LocalAI/core/services/studio/builtins" "github.com/mudler/LocalAI/core/services/studio/session" "github.com/mudler/LocalAI/core/services/studio/workspace" "github.com/mudler/LocalAI/pkg/addons/contract" @@ -34,6 +35,10 @@ type AddonTool struct { // InstalledAt is when the add-on was first installed (RFC 3339, UTC), or // empty. It decides the default provider of a shadowed name. InstalledAt string + // Permission is the capability (contract.ToolPermission) that the sender + // of a connector turn needs to use the tool, or empty. The host sets it + // from the manifest; the core does not know which add-on declares it. + Permission string } // AddonTools is the add-on broker as the chats see it. Both calls run as the @@ -239,14 +244,21 @@ func (s *Service) addonTools(ctx context.Context, a workspace.Actor, rec session tools = append(tools, t) continue } - tools = append(tools, session.Tool{ + tool := session.Tool{ Name: name, Description: d.Description, Schema: schema, // The user and the conversation come from the chat, never from the // arguments the model wrote. Run: func(ctx context.Context, args json.RawMessage) (string, error) { return s.d.AddonTools.Call(ctx, a.Principal, a.ChatID, name, args) }, - }) + } + if d.Permission != "" && rec.Origin == session.OriginConnector { + var ok bool + if tool, ok = s.gateAddonTool(tool, d.Permission, a.ChatID); !ok { + continue + } + } + tools = append(tools, tool) } installed := map[string]string{} for _, d := range defs { @@ -276,3 +288,26 @@ func unavailableTool(addonID, reason string) session.Tool { Run: func(context.Context, json.RawMessage) (string, error) { return msg, nil }, } } + +// gateAddonTool holds a tool of an add-on to the capability it declares (see +// AddonTool.Permission) in a connector chat. The tool is offered only when the +// sender of the turn holds the capability, and a call checks again and records +// the decision, because the grant of a turn can shrink after the tools were +// offered. The second result is false when the tool must not be offered. A chat +// that no connector started has no sender and is not gated here: the write +// mode of the binding and the confirmation of the proposal govern it. +func (s *Service) gateAddonTool(t session.Tool, permission, chat string) (session.Tool, bool) { + allow, audit := s.turnAllow(chat) + c := builtins.Chat{Allow: allow, Audit: audit} + if held, _ := c.Holds(permission); !held { + return t, false + } + run := t.Run + t.Run = func(ctx context.Context, args json.RawMessage) (string, error) { + if err := c.Require(permission); err != nil { + return "", err + } + return run(ctx, args) + } + return t, true +} diff --git a/docs/content/features/chats-connectors.md b/docs/content/features/chats-connectors.md index c7b73e5e8c..1da9db59aa 100644 --- a/docs/content/features/chats-connectors.md +++ b/docs/content/features/chats-connectors.md @@ -512,6 +512,17 @@ or has a role that you granted it. A sensitive capability needs a pairing; a rol never counts. If several people share one turn, the turn gets only what all of them may do. +A tool of an add-on that holds your credential and changes something outside +LocalAI (a comment on an issue, for example) needs `tools.write`. The host reads +this from the add-on's manifest, so it applies to every add-on of that shape. The +sender needs the capability as above; the write mode of the connection and the +confirmation of the change still apply on top of it. + +The agent's grants narrow the result: see Builtin grants in +[Agents](/features/chats-agents/). For `triggers.propose`, `triggers.run`, +`talkback.channels`, `talkback.new_channel` and `files.upload` the sender must hold +the capability and the agent must hold the grant. + The check decides which tools the model gets. A tool that the sender may not use is not offered, so the model cannot be talked into it. If the model calls one anyway, the call is refused with the reason, and the model tells the person. Today this gates the diff --git a/pkg/addons/contract/connector_test.go b/pkg/addons/contract/connector_test.go index 3502722464..9712cca49b 100644 --- a/pkg/addons/contract/connector_test.go +++ b/pkg/addons/contract/connector_test.go @@ -152,3 +152,21 @@ var _ = It("cuts a long name on a character boundary", func() { Expect(utf8.ValidString(got)).To(BeTrue()) Expect(got).To(HaveSuffix(".csv")) }) + +var _ = Describe("the permission of a tool", func() { + It("is tools.write for a tool that is not a read, in an add-on that holds a credential", func() { + for _, kind := range []string{ToolKindWrite, ToolKindSchema, ToolKindCreate, ToolKindAction} { + Expect(ToolPermission(true, ToolDefinition{Kind: kind})).To(Equal(CapToolsWrite), kind) + } + }) + It("is empty for a read, and for any tool of an add-on without a credential", func() { + Expect(ToolPermission(true, ToolDefinition{Kind: ToolKindRead})).To(BeEmpty()) + for _, kind := range []string{ToolKindRead, ToolKindWrite, ToolKindSchema, ToolKindCreate, ToolKindAction} { + Expect(ToolPermission(false, ToolDefinition{Kind: kind})).To(BeEmpty(), kind) + } + }) + It("names a capability in the closed list", func() { + Expect(KnownCapability(CapToolsWrite)).To(BeTrue()) + Expect(SensitiveCapability(CapToolsWrite)).To(BeFalse()) + }) +}) diff --git a/pkg/addons/contract/permissions.go b/pkg/addons/contract/permissions.go index 5867360ac3..acc29712d9 100644 --- a/pkg/addons/contract/permissions.go +++ b/pkg/addons/contract/permissions.go @@ -120,6 +120,19 @@ func ConnectorCapabilities() []CapabilityInfo { return out } +// ToolPermission is the capability that a sender needs to use a tool of an +// add-on in a connector chat, or "" when the tool needs none beyond the +// binding. The rule is by shape, so the host names no add-on: a tool that +// writes, in an add-on that holds a credential for the person's binding, +// changes something outside LocalAI with that person's token, and needs +// CapToolsWrite. credentialed says that the manifest declares credentials. +func ToolPermission(credentialed bool, d ToolDefinition) string { + if credentialed && d.Kind != ToolKindRead { + return CapToolsWrite + } + return "" +} + // ConnectorRole is a role that a kind can give to a sender, such as the admin // of a workspace. The add-on decides who has which role and sends the role IDs // with each message; the owner decides what each role may do.