[ENG-2422] Tell a web turn where credentials go instead of asking for them in chat - #466
Open
tino097 wants to merge 9 commits into
Open
Conversation
…sted one Four strings solicited a chat paste: two base-prompt bullets both surfaces share, and two on the console-less connect path that Cowork desktop gets. On a host with no credential tool, chat was the only way to comply. The base prompt now carries the prohibition alone and names no tool, since where a credential goes differs per host. Recovery is untouched: mode (a) still vaults a value that already reached the conversation, which is the only thing that gets it out of the transcript. Where nothing can store a credential the rule forbids storing it at all, rather than listing stores to avoid. The pod allowlists memorize and create_skill_draft, memory writes are applied and re-sent on later turns, and the relay scrubber matches four shapes that a GitHub PAT, a WordPress application password and an SMTP password all miss, so a stored value would turn one exposed trace into permanent exposure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… could The pod passed no system-prompt suffix at all, so every referral shipped into the harness suffix chain stopped at the process that refuses org turns. The model reached the end of an answer, found it had no credential tool and no place to name, and users pasted secrets rather than get nothing. The suffix names the chat-side Connectors entry and nothing else. It names no connector, because which ones a deployment offers comes from auth's OAuth configuration that the pod cannot see, and claiming one either way risks sending the user after something absent. It offers no other surface as a workaround: this turn's connections are only what was listed in the oauth block, so a credential saved elsewhere is unreachable from here. The allowlist assertion compares against a literal set rather than the constant, which the neighbouring test does and so never trips on an addition. Any new tool now fails it, which is the point: a credential-capture tool would make the referral untrue, and any other is a new place a pasted value can land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shared description tail still said to ask for missing pieces in a later turn, so both tool variants carried a solicitation after the head text stopped carrying one. The scrub-marker note still said to pass the actual secret values instead, in the same tool result that now says not to ask for them; the values behind a marker cannot be resolved from the vault at that point, so it now says that and points at the credential flow. The base prompt claimed the flow is named elsewhere in the prompt. On desktop it is not: the harness attaches its workflow text to a skill bundle, so it reaches the model as a tool rather than as prompt text. It now says either. Both description tests asserted the single literal that had been deleted while naming a property they did not check, which is how the tail survived. They now scan every ask-shaped sentence and require it to be negated, which catches a reworded solicitation and catches the tail. Negation strips apostrophes first, since a word boundary never fires inside "don't". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rity The suffix named a Connectors entry beside the chat. cowork's sidebar calls it Connect Apps and Data, and reads Connected Apps and Data once something is connected; Connectors titles a different page whose web branch saves personal tokens through a route with no org-mode handling, into a vault no turn reads. Sending a user there would have reproduced this dead end one step later, while the suffix's own closing sentence denied it. The suffix also refused to name another surface at all, which the ticket asks for the opposite of: state parity plainly, either it works here or say where it must happen and how. The web UI already points at the desktop app in two places, so saying nothing had the agent contradict the screen. It now says it, with the part the UI leaves out: a credential added there is not readable from this conversation, so the work moves rather than the credential. The test pinned the absence of that referral, which made it a test of the choice rather than of the requirement. It now pins the referral and its caveat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The base prompt and the connector tool descriptions are shared with the CLI and the desktop, and this ticket is about web users, so they go back to what they were. Only the cloud-turn suffix remains. The suffix now carries the whole rule rather than half of it. It has to: with the shared prompt reverted, nothing else on this surface forbids asking for a credential, tells the user to rotate one that arrives anyway, or forbids storing it. That last clause is the one that cannot be dropped, because memorize and create_skill_draft are both allowlisted here, memory writes are applied org-side and replayed on later turns, and the relay scrubber matches four shapes that a GitHub PAT, a WordPress application password and an SMTP password all miss. Two costs, both deliberate. The shared prompt still lists credentials among the things to ask the user for, so the suffix countermands it in words rather than relying only on being appended last. And the solicitation on the desktop path stays as it is, though five users pasted live credentials there in a single week, so it wants its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b1e1845af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # line is derived by the session from the provider's response. | ||
| system_prompt_context=SystemPromptContext( | ||
| runtime_context=build_runtime_context(settings), | ||
| suffix=_CREDENTIAL_CONTEXT, |
There was a problem hiding this comment.
When ANTON_ROUTER_ENABLED=true, text turns first go through gate_turn, whose independent _THALAMUS_SYSTEM_PROMPT never receives this SystemPromptContext; an ACTION_RESPOND result is returned directly without invoking the planning model. A vague setup request that the router answers with a clarifying question—or a pasted credential that the existing scrubber does not recognize—can therefore still cause the assistant to solicit or repeat a secret despite this new policy. Pass the credential policy to the gate or prevent direct routing for these cloud turns.
Useful? React with 👍 / 👎.
| # line is derived by the session from the provider's response. | ||
| system_prompt_context=SystemPromptContext( | ||
| runtime_context=build_runtime_context(settings), | ||
| suffix=_CREDENTIAL_CONTEXT, |
There was a problem hiding this comment.
When a turn has org or project memory, ChatSystemPromptBuilder.build appends memory_context after this suffix (prompt_builder.py:213-216), contrary to the placement assumption above. A remembered Always rule instructing the agent to request or store credentials consequently appears later than the safety policy, while the policy only explicitly overrides “earlier” instructions; this can preserve the credential-leak behavior for affected conversations. Ensure this guard is rendered after the volatile memory tail, or otherwise give it precedence over remembered rules.
Useful? React with 👍 / 👎.
… came before The remembered-rules snapshot renders after the suffix, so a rule asking for an API key was never countermanded by a guard that claimed precedence only over earlier instructions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the placement note A recalled skill body is the organization's own text and arrives as tool output, so a guard bounded by this prompt did not reach it. The comment claiming the suffix renders last was disproved by the commit before this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Artifact delivery and the credential rule both set the single suffix field. They are joined into one constant so the whole of what this surface says can be pinned by a test, and neither block depends on its position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ents it The builder's volatile tail is not a cached boundary: anton places no cache_control and the gateway marks the whole system string as one block, so where the guard sits inside it has no cache consequence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://linear.app/mindsdb/issue/ENG-2422/a-web-user-cannot-create-a-connector-credential-at-all-so-the-only
What changed
one
remembered-rules snapshot is appended after the suffix, so an org rule asking
for an API key was never countermanded by a guard claiming precedence over
earlier text alone
recall_skillisallowlisted and the skill tree is the tenant's, so a user-authored skill body
saying to ask for a key arrives as tool output, outside a guard bounded by
"in this prompt"
The fifth commit narrows the first four; the last two answer review findings. The net diff against
stagingis twofiles,
anton/cloud_turn/session.pyandtests/test_cloud_turn_session.py:the shared base prompt and the connector tool descriptions are byte-identical
to base, so the CLI and the desktop are untouched. Reviewing the net diff is
easier than reading the commits in order.
Notes for the reviewer
reverted, nothing else on this surface forbids asking for a credential, tells
the user to rotate one that arrives anyway, or forbids storing it.
memorizeandcreate_skill_draftare both allowlisted in the pod, memory writes areapplied org-side and replayed on later turns, and the relay's scrubber
matches four shapes that a GitHub PAT, a WordPress application password and
an SMTP password all miss. Without it, "get the secret out of the transcript"
reads as "store it somewhere else", which would turn one exposed trace into
permanent exposure.
position. The base prompt still lists credentials among the things to ask
the user for. The suffix does not render last:
ChatSystemPromptBuilder.buildappends the relevance-filtered memory snapshot after it, and a recalled skill
body never passes through the prompt at all. So the text claims precedence
over every channel explicitly rather than relying on placement. A test
asserts the shared invitation is still present, so if it is ever removed this
override gets revisited rather than lingering.
it is byte-identical every turn. It lengthens the cached block, which is
written once and then read at about 0.1x, and adds neither invalidator from
the prompt-cache investigation. Note for anyone reasoning about placement
here: the ordering above is not a caching argument. anton places no
cache_control, and the gateway wraps the whole system string in one blockwith a single breakpoint at its end, so position inside
systemhas no cacheconsequence either way.
anton/tools.pystillsays to ask for credentials in chat on the console-less path, which is Cowork
desktop, and ENG-1849 observed five users pasting live credentials there in a
single week. Deliberately out of scope here; it wants its own change.
Connectorsalsotitles the Code page whose non-Electron branch saves personal tokens through
/connectors/connections/validate-and-save, a route with no org-modehandling, into a vault no pod turn reads. Pointing a web user there would
reproduce this dead end one step later. A rename in cowork has no test on
this side to catch it.
on the grounds that a credential added there is unreachable from a web turn.
The ticket asks for parity stated plainly and the web UI already points at
the desktop app in two places, so silence had the agent contradict the
screen. The caveat the UI omits rides along in the prompt.
surfaceread.build_cloud_chat_sessionhas one production caller,the headless entrypoint, so the suffix is web-only by construction.
Branching on
surfacewould break the relay's stated invariant that thetraceblock is observability only.met. Criterion 1 is met only for the connectors auth actually supports.
Criterion 2 needs transcript-side redaction and criterion 5 is now ENG-2661,
which has to close before the ticket can. This PR should not close it.
/connectors/connections/saveand/validate-and-savehave noorg_modebranch although five siblings in the same module do, and live callers exist
in the shell-agnostic renderer tree. An org web user can store a credential
nothing can read. Worth its own ticket.
How to test
system prompt: the suffix names
Connect Apps and Data, names no connector,carries the desktop referral with its caveat, and forbids storing a value.
git diff staging..HEAD -- anton/core/llm/prompts.py anton/tools.pyis empty.Checks
uv run --group dev pytest tests/ --ignore=tests/e2euv run --group dev pytest tests/e2e/Both re-run at the head commit. Every new assertion was also confirmed to
invert at the base revision, so they fail on the original bug rather than
passing vacuously.
🤖 Generated with Claude Code