Skip to content

fix(security): keep security explanations private and stop their tool calls - #471

Draft
johnmatthewtennant wants to merge 9 commits into
jtennant/gordon/security-explanation-effortfrom
jtennant/gordon/security-explanation-private
Draft

johnmatthewtennant wants to merge 9 commits into
jtennant/gordon/security-explanation-effortfrom
jtennant/gordon/security-explanation-private

Conversation

@johnmatthewtennant

@johnmatthewtennant johnmatthewtennant commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This fixes a bug: the hidden session that generates a security explanation is meant to have no tools and to stay out of the app, but it can still run a tool and its updates reach the visible chat store.

inferSecurityExplanation removes every session extension so the model has nothing to act with. Goose injects load_skill into every session itself, though, so the model is still offered that tool and the skills it can load. The session is also never marked private, so:

  • a load_skill call in the default approve mode raises a permission request through the app's normal permission handler, for a session nobody can see;
  • the session's config updates from setModel go to the chat store.

This PR:

  • marks the session private as soon as newSession returns. Its later updates are dropped and its permission requests are cancelled, as voice drafts already do.
  • requires chat mode before the prompt. In chat mode Goose skips every tool call, including injected ones such as load_skill. If the mode can't be set, the explanation isn't prompted, and the failure is logged.
  • lifts the private mark when a session with an id that is already marked is created, forked, imported or loaded. Goose allocates session ids from the highest remaining one, so the next session after a deleted private session can take its id, including one created by another client. Without this, the private mark would drop that session's updates and cancel its permission requests, and voice drafts already hit the same reuse.

load_skill is still offered to the model. Removing it needs a Goose change; with this PR a call to it is skipped.

Related issue

None found.

Testing

Red/green through the production path: Berd's inferSecurityExplanation and its real ACP connection ran against the pinned Goose goose serve in approve mode, with one user skill installed. The model was a local stub that answers the explanation prompt with a load_skill call. The harness recorded what reached the app's permission and chat-store handlers, and it searched and listed sessions while the explanation ran.

Before After
load_skill call permission request shown by the app handler skipped by chat mode, no permission request
Explanation session updates in the chat store 4 (including 2 config_option_update) 2, both sent before newSession returns
Session search and list during the explanation not found not found
Explanation returned returned
Next visible chat, which reuses the explanation's id all updates, permission request shown all updates, permission request shown

Unit tests in inferExplanation.test.ts cover the contract (all fail before this change):

  • the session is marked private before setModel;
  • chat mode is set before the prompt;
  • when chat mode can't be set, nothing is prompted, the session is deleted and the failure is logged.

acpBackends.test.ts covers the id reuse: a created, forked, imported or loaded session that has a private id receives its updates and permission requests (fails without the fix).

Not verified: opening an alert without a detector-written explanation in a packaged build. The harness above calls inferSecurityExplanation with the app's real permission and chat-store handlers rather than going through the alert UI.

🤖 Generated with Claude Code

johnmatthewtennant and others added 2 commits November 5, 2026 13:48
…leak updates

Goose injects load_skill into every session, so removing extensions
leaves a tool the model can call. Its permission request and the
session's config updates reach the visible app handlers.

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Mark the hidden session private as soon as it exists, so its updates
stay out of the chat store and its permission requests are cancelled,
and require chat mode before the prompt, so no tool call runs.

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
@johnmatthewtennant johnmatthewtennant changed the title fix(security): keep security explanations private and tool-free fix(security): keep security explanations private and stop their tool calls Nov 5, 2026
johnmatthewtennant and others added 7 commits November 5, 2026 13:53
Goose allocates session ids from the highest remaining one, so deleting
a private session lets the next chat take its id, and the private
fence then drops that chat's updates and cancels its permissions.

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Goose allocates session ids from the highest remaining one, so a chat
created after a private session is deleted can take that id. The
private fence then dropped the chat's updates and cancelled its
permission requests. newSession now reveals the id it returns.

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
…ate id

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
…ate id

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Another client, such as a scheduled job, can create a session with a
deleted private session's id. Loading it means the app shows it.

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
…n/security-explanation-private

Signed-off-by: John Matthew Tennant <jtennant@block.xyz>

Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* jtennant/gordon/security-explanation-effort:
  fix(connections): remove retired Block Academy provider (#475)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant