diff --git a/AGENTS.md b/AGENTS.md index 55ce7e55b..ed3b4fadd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -163,7 +163,7 @@ The Bot team: ## Hard rules 1. **Build on DSH; never patch it.** Bot features use the documented plugin APIs. The desktop app starts DSH only through its documented entry points (`@deepseek-ai/dsh/profile-boot`, the `dsh` CLI). Never patch DeepSeek Harness or depend on files inside a DSH install. If something is missing in DSH, stop and say so; do not work around it with a patch. -2. **No secrets.** Never write, print or commit API keys, tokens, cookies or `DSH_HOME` contents. Use the mock model (`core/dev/mock-llm.mjs`) for tests; it needs no key. +2. **No secrets.** Never write, print or commit API keys, tokens, cookies or `DSH_HOME` contents. Use the mock model (`core/dev/mock-llm.mjs`) for tests; it needs no key. A shell that holds no key this turn says why in `DSH_SECRETS_WITHHELD` (`NAME:reason`, never a value, #651): read the reason instead of assuming the owner did not configure it. 3. **Only original material.** Do not copy code, text, icons, logos or graphics from other products or websites. 4. **The core is ds-bot + patches.** Change `core/`, then run `node scripts/ds-bot-patches.mjs generate` and `check`, and commit `core/` and `patches/` together. CI fails if `ds-bot/` differs from the pin or `core/` differs from `ds-bot/` + patches. 5. **`client.js` is generated.** Change `core/src/client/`, then run `npm run build` in `core/` and commit both files. Never edit `client.js` by hand. diff --git a/changelog.d/651-secrets-withheld.fixed.md b/changelog.d/651-secrets-withheld.fixed.md new file mode 100644 index 000000000..dde3bb054 --- /dev/null +++ b/changelog.d/651-secrets-withheld.fixed.md @@ -0,0 +1 @@ +A Bot's shell now says which saved key it withheld and why, instead of looking like the key was never saved: `DSH_SECRETS_WITHHELD` lists `NAME:reason` (never a value), `list_secrets` and `request_secret` name the same reason, and a key the owner scoped to one Bot still reaches that Bot's own scheduled turn while it is shared ([#651](https://github.com/ThinkFlowLab/FlyBot/issues/651)). diff --git a/core/src/host/secrets.js b/core/src/host/secrets.js index 47f41fb87..0b4365d46 100644 --- a/core/src/host/secrets.js +++ b/core/src/host/secrets.js @@ -7,7 +7,7 @@ import { chmod, mkdir, readFile, rename, unlink, writeFile } from 'node:fs/promi import { join } from 'node:path' import { defineTool } from '@deepseek-ai/dsh-tools' import { output } from './tools.js' -import { OTHER, settled } from './turn-origin.js' +import { OTHER, SCHEDULED, OWNER, settled, turnOrigin } from './turn-origin.js' export const SECRET_NAME = /^[A-Z][A-Z0-9_]{0,63}$/ export const ENV_PREFIX = 'DSH_SECRET_' @@ -21,6 +21,17 @@ const MAX_REQUESTS = 20 const USED_SAVE_MS = 5000 export const envName = name => `${ENV_PREFIX}${name}` +// A shell this turn cannot use a key in says so instead of looking empty (#601, #651): names and a +// reason, never a value, so "you did not configure it" and "it was withheld" stop looking the same. +// The reasons are a closed set; a caller that adds one adds it here. +export const WITHHELD_VAR = 'DSH_SECRETS_WITHHELD' +export const WITHHELD_REASONS = Object.freeze({ + REMOTE: 'remote-bot', + NOT_OWNER: 'turn-not-owner', + SHARED_SCHEDULED: 'shared-bot-scheduled', + SHARED_ROOM: 'shared-room', + UNKNOWN_TURN: 'unknown-turn', +}) export const canUse = (secret, botId) => secret !== undefined && botId !== undefined && (secret.scope === 'all' || (Array.isArray(secret.scope) && secret.scope.includes(botId))) @@ -402,7 +413,11 @@ export function install(rt) { const name = normalizeName(args.name) if (!SECRET_NAME.test(name)) return 'Name the key with capitals, digits, and underscores, starting with a letter (for example GITHUB_TOKEN).' const secret = secrets.get(name) - if (canUse(secret, bot.id)) return `${name} is already saved and you may use it: ${usage(name)}. You cannot see the value; never print it.` + if (canUse(secret, bot.id)) { + const reason = withheldReason(secret, id, bot.id, bot) + if (reason !== undefined) return `${name} is saved for this Bot but withheld this turn: ${reason}. A key is only in the shell on the owner's own turn. You cannot see the value; never print it.` + return `${name} is already saved and you may use it: ${usage(name)}. You cannot see the value; never print it.` + } if (rt.groupOwners.has(id)) return 'Secret cards only work in your own chat. Tell the group you will ask the user in your own chat, and call request_secret there.' if (!rt.chatOwners.has(id)) return 'Secret cards only work in your current own chat.' const readOnly = rt.readOnly() @@ -445,9 +460,16 @@ export function install(rt) { const self = callerOf(exec) if (rt.botOf(self) === undefined) return 'Only Bots on the team can use list_secrets.' if (secrets.size === 0) return 'No keys are saved yet. Ask for one with request_secret.' + const bot = rt.botOf(self) + // #651: say why a key is not in this turn's shell. A name and a reason, never a value. + const use = (secret) => { + if (!canUse(secrets.get(secret.name), self)) return 'you may not use it; ask with request_secret' + const reason = bot === undefined ? undefined : withheldReason(secrets.get(secret.name), exec.agent?.id, self, bot) + return reason === undefined ? `you may use it as $${envName(secret.name)}` : `withheld this turn: ${reason} — ask the user for the owner's turn` + } return [ 'Saved keys (values are never shown):', - ...secretsView().map(secret => `- ${secret.name}${secret.purpose ? ` — ${secret.purpose}` : ''} — for ${scopeText(secret.scope)} — ${canUse(secrets.get(secret.name), self) ? `you may use it as $${envName(secret.name)}` : 'you may not use it; ask with request_secret'}`), + ...secretsView().map(secret => `- ${secret.name}${secret.purpose ? ` — ${secret.purpose}` : ''} — for ${scopeText(secret.scope)} — ${use(secret)}`), ].join('\n') }, })) @@ -497,16 +519,37 @@ export function install(rt) { return rt.sharedWith(bot.id).length === 0 } + // Why this key is not in the shell for the turn that is running, or undefined when it is. A key + // the owner scoped to this Bot by name (scope []) is this Bot's own: it reaches the Bot's + // scheduled turn even while the Bot is shared, because the owner said which Bot may use it. + // scope 'all' is "you did not pick a Bot", so it keeps the fail-closed rule (#651 B). + const withheldReason = (secret, sessionId, botId, bot) => { + if (bot.remote !== undefined) return WITHHELD_REASONS.REMOTE + const origin = turnOrigin(rt, sessionId) + if (origin === OTHER) return WITHHELD_REASONS.NOT_OWNER + if (ownersTurn(sessionId, bot)) return undefined + const explicitForThisBot = Array.isArray(secret.scope) && secret.scope.includes(botId) + if (origin === SCHEDULED) return explicitForThisBot ? undefined : WITHHELD_REASONS.SHARED_SCHEDULED + if (origin === OWNER) return WITHHELD_REASONS.SHARED_ROOM + return WITHHELD_REASONS.UNKNOWN_TURN + } + function resolveEnv(execution) { const sessionId = execution?.agent?.session?.header?.id ?? execution?.agent?.id const botId = rt.selfOf(sessionId) const bot = rt.botOf(botId) - if (bot === undefined || bot.remote !== undefined || !ownersTurn(sessionId, bot)) return {} + if (bot === undefined) return {} const command = typeof execution.arguments?.command === 'string' ? execution.arguments.command : '' const values = {} + const withheld = [] let used = false for (const secret of secrets.values()) { if (!canUse(secret, botId)) continue + const reason = withheldReason(secret, sessionId, botId, bot) + if (reason !== undefined) { + withheld.push(`${secret.name}:${reason}`) + continue + } values[envName(secret.name)] = secret.value if (command.includes(envName(secret.name))) { secret.lastUsedAt = Date.now() @@ -514,6 +557,7 @@ export function install(rt) { } } if (used) noteUsed() + if (withheld.length) values[WITHHELD_VAR] = withheld.join(',') return values } diff --git a/core/src/host/turn-origin.js b/core/src/host/turn-origin.js index 425b48981..52e35c748 100644 --- a/core/src/host/turn-origin.js +++ b/core/src/host/turn-origin.js @@ -105,6 +105,13 @@ export function combine(a, b) { return a === SCHEDULED || b === SCHEDULED ? SCHEDULED : OWNER } +// The verdict the running turn recorded, before `settled` narrows it. `scheduled` is the one a caller +// cannot get anywhere else: on a shared Bot it is 'other' as a permission, and still 'scheduled' as +// a fact, which is what a caller needs to say why a turn is not the owner's (#651). +export function turnOrigin(rt, sessionId, entry = rt.live?.get(sessionId)) { + return entry?.turn === undefined ? undefined : entry.origin +} + // The verdict that holds right now for the turn a Session is running: 'owner', 'other', or // undefined when the Session runs no turn or no message of it carried a verdict. `entry` is the // Session's `rt.live` record; the checks here read the team as it is at this moment, so a person diff --git a/core/test/secrets.test.js b/core/test/secrets.test.js index 2f3120c8d..65307c310 100644 --- a/core/test/secrets.test.js +++ b/core/test/secrets.test.js @@ -276,7 +276,7 @@ test('a key saved for no Bot by the old default is moved to all Bots once; an ex const kept = fresh() const picked = fresh() const at = { createdAt: 1, updatedAt: 1, requestedBy: null, purpose: '' } - const bench = start({ secrets: [ + const bench = await seeded({ secrets: [ { name: 'GITHUB_TOKEN', value: github, scope: [], ...at, purpose: 'GitHub connector' }, { name: 'NOBODY_KEY', value: kept, scope: [], scopeSet: true, ...at }, { name: 'WRITER_KEY', value: picked, scope: ['b2'], ...at }, @@ -313,7 +313,7 @@ test('a key for all Bots stays out of a shared Bot\'s shell when the turn is not t.after(bench.stop) await bench.call('secret-set', { name: 'GITHUB_TOKEN', value }) assert.deepEqual(bench.env('b2'), { DSH_SECRET_GITHUB_TOKEN: value }) - assert.deepEqual(bench.env('c3'), {}) + assert.deepEqual(bench.env('c3'), { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:unknown-turn' }) }) test('tool results and model requests lose the values', async (t) => { @@ -351,3 +351,79 @@ test('tool results and model requests lose the values', async (t) => { stream({ ...options, system: 'clean', messages: [] }, next) assert.equal(passed.length, 2) }) + +// #651: a shell says which key was withheld and why, never the value. Before this, `resolveEnv` +// returned `{}` and "the owner did not configure it" and "it was withheld" looked the same. +const SHARED = { ...TEAM, grants: { b2: [{ grantee: '@bob.home', name: 'Bob' }] } } +const saved = scope => [{ name: 'GITHUB_TOKEN', value: 'ghp_testvalue_123', scope, purpose: 'GitHub' }] +// The seed is read asynchronously, so one state read settles it before the shell is asked. +const seeded = async options => { const bench = start(options); await bench.view(); return bench } + +test('#651: the owner’s own turn gets the key and says nothing about withholding', async (t) => { + const bench = await seeded({ secrets: saved('all') }) + t.after(bench.stop) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user' } } }) + const env = bench.env('b2') + assert.deepEqual(env, { DSH_SECRET_GITHUB_TOKEN: 'ghp_testvalue_123' }) + assert.equal(env.DSH_SECRETS_WITHHELD, undefined) +}) + +test('#651: someone else’s turn names the key and the reason, and carries no value', async (t) => { + const bench = await seeded({ secrets: saved('all') }) + t.after(bench.stop) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user', authorId: 'ann' } } }) + const env = bench.env('b2') + assert.deepEqual(env, { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:turn-not-owner' }) + assert.ok(!JSON.stringify(env).includes('ghp_testvalue_123'), 'no value anywhere in the shell env') +}) + +test('#651: a friend’s Bot says remote-bot, and a host with no key says nothing at all', async (t) => { + const friend = { + ...TEAM, + bots: { ...TEAM.bots, f9: { id: 'f9', sessionId: 'f9', name: 'Pal', role: '友', instructions: '', createdAt: 9, remote: { handle: '@pal.ann.home' } } }, + } + const bench = await seeded({ secrets: saved('all'), team: friend }) + t.after(bench.stop) + const env = bench.env('f9') + assert.deepEqual(env, { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:remote-bot' }) + assert.ok(!JSON.stringify(env).includes('ghp_testvalue_123')) + + // Nothing saved: the same `{}` as before, and the tool says so in those words (#651). + const empty = start() + t.after(empty.stop) + assert.deepEqual(empty.env('b2'), {}) + const listed = await empty.tool('list_secrets', {}, 'b2') + assert.match(listed.text, /No keys are saved yet/) + assert.ok(!listed.text.includes('withheld')) +}) + +test('#651: a shared Bot’s scheduled turn withholds scope "all" and keeps it for the Bot it names', async (t) => { + const sharedAll = await seeded({ secrets: saved('all'), team: SHARED }) + t.after(sharedAll.stop) + sharedAll.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) + sharedAll.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'schedule' } } }) + assert.deepEqual(sharedAll.env('b2'), { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:shared-bot-scheduled' }) + + // The owner said which Bot may use this key, so that Bot's own scheduled turn still has it. + const sharedThis = await seeded({ secrets: saved(['b2']), team: SHARED }) + t.after(sharedThis.stop) + sharedThis.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) + sharedThis.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'schedule' } } }) + assert.deepEqual(sharedThis.env('b2'), { DSH_SECRET_GITHUB_TOKEN: 'ghp_testvalue_123' }) +}) + +test('#651: list_secrets and request_secret give the same reason, and never a value', async (t) => { + const bench = await seeded({ secrets: saved('all') }) + t.after(bench.stop) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) + bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user', authorId: 'ann' } } }) + const listed = await bench.tool('list_secrets', {}, 'b2') + assert.match(listed.text, /GITHUB_TOKEN/) + assert.match(listed.text, /withheld this turn: turn-not-owner/) + assert.ok(!listed.text.includes('ghp_testvalue_123')) + const asked = await bench.tool('request_secret', { name: 'GITHUB_TOKEN', purpose: 'GitHub' }, 'b2') + assert.match(asked.text, /withheld this turn: turn-not-owner/) + assert.ok(!asked.text.includes('ghp_testvalue_123')) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 32f2a8952..72c51a660 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -209,6 +209,10 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi A key the owner saves is for **all of the owner's Bots** (`scope: 'all'`) unless the owner narrows it: a card's "Only ", the Secrets page's *Chosen Bots* or *No Bot*. That holds for a connector key too (`GITHUB_TOKEN`, `COMPOSIO_API_KEY`): `connector-connect` no longer passes `scope: []`, and a reconnect keeps the scope the key already has. A scope the owner chose is stored with `scopeSet: true` (`secret-update`, `secret-set` with a `scope`, a card's "Only ", and a deleted Bot leaving a picked list), so an empty list stays "No Bot". On load, an empty list **without** `scopeSet` is the old connector default and becomes `'all'` once (written back by the host that holds the team; a reading host widens it in memory only). An empty list the owner picked before #545 carries no marker and is widened too: there is no way to tell it apart. `'all'` is the owner's own use, the same rule as the connector tools: a Bot's shell gets the keys only on the owner's own turn (`settled` in `turn-origin.js`; a turn someone else started gets none), and outside a known turn only while the Bot is shared with nobody (`rt.sharedWith`); a friend's Bot (`remote`) never does. The boundary as measured on 2026-10-21 (#581): a Bot's **own** scheduled turn already counts as the owner's while that Bot is not shared, so a routine whose timer fires in its own Session gets the keys; a **group** round inherits the initiator's verdict, so a round a *shared* Bot opened on a scheduled turn reaches every member as `other` — the shared initiator by design, and the unshared members only because the relay carries that verdict — and none of them gets the keys. The routines avoid it by scheduling themselves in their own, unshared Sessions; that is a runtime route, no code change (#581, closed). A capability (`canUseSecret`) mounts for the whole Agent, so on a Bot shared with anyone it gets only a key whose list names that Bot, never an `'all'` key. Owner decision 2026-10-20 (#359, AGENTS.md hard rule 4). +### A withheld key says why (#651) + +A key that is not in this turn's shell used to look exactly like a key the owner never saved: `resolveEnv` returned `{}` and the Bot reported "no key configured". Now `resolveEnv` writes `DSH_SECRETS_WITHHELD` — a comma-separated `NAME:reason` list, never a value — with one reason from a closed set: `remote-bot` (a friend's Bot), `turn-not-owner` (the turn carries someone else's words), `shared-bot-scheduled` (a shared Bot's own scheduled turn holding an `'all'` key), `shared-room` (a room turn on a shared Bot) and `unknown-turn` (no turn recorded). `withheldReason` is the single source: `resolveEnv`, `list_secrets` and the `request_secret` answer all read it, so the shell and the tool text name the same reason. To tell a scheduled turn apart from a room turn, `turnOrigin(rt, sessionId)` returns the turn's recorded verdict before `settled` narrows it: on a shared Bot `settled` answers `other` for both, while `turnOrigin` still says `scheduled`. A key whose list names this Bot (`scope: []`) therefore reaches that Bot's own scheduled turn even while it is shared; `scope: 'all'` keeps the fail-closed rule, because it means "the owner did not pick a Bot". Tests: the four `#651` cases in `core/test/secrets.test.js`. + ### Per-Bot connector access (#335, #369) The connector tools are mounted once for the whole DSH profile, so who may use them through a Bot is decided in `src/host/connector-access.js`, from two things: **whose turn it is** and the Bot's two switches. diff --git a/patches/ds-bot/0014-flybot-branding.patch b/patches/ds-bot/0014-flybot-branding.patch index f9521b353..2533b5425 100644 --- a/patches/ds-bot/0014-flybot-branding.patch +++ b/patches/ds-bot/0014-flybot-branding.patch @@ -491,10 +491,28 @@ diff --git a/src/host/secrets.js b/src/host/secrets.js import { join } from 'node:path' import { defineTool } from '@deepseek-ai/dsh-tools' import { output } from './tools.js' -+import { OTHER, settled } from './turn-origin.js' ++import { OTHER, SCHEDULED, OWNER, settled, turnOrigin } from './turn-origin.js' export const SECRET_NAME = /^[A-Z][A-Z0-9_]{0,63}$/ export const ENV_PREFIX = 'DSH_SECRET_' +@@ -20,6 +20,17 @@ const MAX_REQUESTS = 20 + const USED_SAVE_MS = 5000 + + export const envName = name => `${ENV_PREFIX}${name}` ++// A shell this turn cannot use a key in says so instead of looking empty (#601, #651): names and a ++// reason, never a value, so "you did not configure it" and "it was withheld" stop looking the same. ++// The reasons are a closed set; a caller that adds one adds it here. ++export const WITHHELD_VAR = 'DSH_SECRETS_WITHHELD' ++export const WITHHELD_REASONS = Object.freeze({ ++ REMOTE: 'remote-bot', ++ NOT_OWNER: 'turn-not-owner', ++ SHARED_SCHEDULED: 'shared-bot-scheduled', ++ SHARED_ROOM: 'shared-room', ++ UNKNOWN_TURN: 'unknown-turn', ++}) + export const canUse = (secret, botId) => secret !== undefined && botId !== undefined + && (secret.scope === 'all' || (Array.isArray(secret.scope) && secret.scope.includes(botId))) + @@ -81,7 +81,9 @@ export function install(rt) { const { ctx, home, state } = rt const path = join(home, 'secrets.json') @@ -601,7 +619,38 @@ diff --git a/src/host/secrets.js b/src/host/secrets.js } } -@@ -465,9 +465,21 @@ export function install(rt) { +@@ -380,7 +380,11 @@ export function install(rt) { + const name = normalizeName(args.name) + if (!SECRET_NAME.test(name)) return 'Name the key with capitals, digits, and underscores, starting with a letter (for example GITHUB_TOKEN).' + const secret = secrets.get(name) +- if (canUse(secret, bot.id)) return `${name} is already saved and you may use it: ${usage(name)}. You cannot see the value; never print it.` ++ if (canUse(secret, bot.id)) { ++ const reason = withheldReason(secret, id, bot.id, bot) ++ if (reason !== undefined) return `${name} is saved for this Bot but withheld this turn: ${reason}. A key is only in the shell on the owner's own turn. You cannot see the value; never print it.` ++ return `${name} is already saved and you may use it: ${usage(name)}. You cannot see the value; never print it.` ++ } + if (rt.groupOwners.has(id)) return 'Secret cards only work in your own chat. Tell the group you will ask the user in your own chat, and call request_secret there.' + if (!rt.chatOwners.has(id)) return 'Secret cards only work in your current own chat.' + const readOnly = rt.readOnly() +@@ -423,9 +423,16 @@ export function install(rt) { + const self = callerOf(exec) + if (rt.botOf(self) === undefined) return 'Only Bots on the team can use list_secrets.' + if (secrets.size === 0) return 'No keys are saved yet. Ask for one with request_secret.' ++ const bot = rt.botOf(self) ++ // #651: say why a key is not in this turn's shell. A name and a reason, never a value. ++ const use = (secret) => { ++ if (!canUse(secrets.get(secret.name), self)) return 'you may not use it; ask with request_secret' ++ const reason = bot === undefined ? undefined : withheldReason(secrets.get(secret.name), exec.agent?.id, self, bot) ++ return reason === undefined ? `you may use it as $${envName(secret.name)}` : `withheld this turn: ${reason} — ask the user for the owner's turn` ++ } + return [ + 'Saved keys (values are never shown):', +- ...secretsView().map(secret => `- ${secret.name}${secret.purpose ? ` — ${secret.purpose}` : ''} — for ${scopeText(secret.scope)} — ${canUse(secrets.get(secret.name), self) ? `you may use it as $${envName(secret.name)}` : 'you may not use it; ask with request_secret'}`), ++ ...secretsView().map(secret => `- ${secret.name}${secret.purpose ? ` — ${secret.purpose}` : ''} — for ${scopeText(secret.scope)} — ${use(secret)}`), + ].join('\n') + }, + })) +@@ -465,14 +465,47 @@ export function install(rt) { return ctx.llm.stream({ ...options, messages: fresh, ...(system !== undefined ? { system } : {}) }) }) @@ -614,6 +663,21 @@ diff --git a/src/host/secrets.js b/src/host/secrets.js + if (verdict !== undefined) return verdict !== OTHER + return rt.sharedWith(bot.id).length === 0 + } ++ ++ // Why this key is not in the shell for the turn that is running, or undefined when it is. A key ++ // the owner scoped to this Bot by name (scope []) is this Bot's own: it reaches the Bot's ++ // scheduled turn even while the Bot is shared, because the owner said which Bot may use it. ++ // scope 'all' is "you did not pick a Bot", so it keeps the fail-closed rule (#651 B). ++ const withheldReason = (secret, sessionId, botId, bot) => { ++ if (bot.remote !== undefined) return WITHHELD_REASONS.REMOTE ++ const origin = turnOrigin(rt, sessionId) ++ if (origin === OTHER) return WITHHELD_REASONS.NOT_OWNER ++ if (ownersTurn(sessionId, bot)) return undefined ++ const explicitForThisBot = Array.isArray(secret.scope) && secret.scope.includes(botId) ++ if (origin === SCHEDULED) return explicitForThisBot ? undefined : WITHHELD_REASONS.SHARED_SCHEDULED ++ if (origin === OWNER) return WITHHELD_REASONS.SHARED_ROOM ++ return WITHHELD_REASONS.UNKNOWN_TURN ++ } + function resolveEnv(execution) { - const botId = rt.selfOf(execution?.agent?.session?.header?.id ?? execution?.agent?.id) @@ -621,10 +685,29 @@ diff --git a/src/host/secrets.js b/src/host/secrets.js + const sessionId = execution?.agent?.session?.header?.id ?? execution?.agent?.id + const botId = rt.selfOf(sessionId) + const bot = rt.botOf(botId) -+ if (bot === undefined || bot.remote !== undefined || !ownersTurn(sessionId, bot)) return {} ++ if (bot === undefined) return {} const command = typeof execution.arguments?.command === 'string' ? execution.arguments.command : '' const values = {} ++ const withheld = [] let used = false + for (const secret of secrets.values()) { + if (!canUse(secret, botId)) continue ++ const reason = withheldReason(secret, sessionId, botId, bot) ++ if (reason !== undefined) { ++ withheld.push(`${secret.name}:${reason}`) ++ continue ++ } + values[envName(secret.name)] = secret.value + if (command.includes(envName(secret.name))) { + secret.lastUsedAt = Date.now() +@@ -480,6 +480,7 @@ export function install(rt) { + } + } + if (used) noteUsed() ++ if (withheld.length) values[WITHHELD_VAR] = withheld.join(',') + return values + } + @@ -499,7 +499,7 @@ export function install(rt) { try { unregister = sub.shellEnv.register({ @@ -928,7 +1011,7 @@ diff --git a/test/secrets.test.js b/test/secrets.test.js + const kept = fresh() + const picked = fresh() + const at = { createdAt: 1, updatedAt: 1, requestedBy: null, purpose: '' } -+ const bench = start({ secrets: [ ++ const bench = await seeded({ secrets: [ + { name: 'GITHUB_TOKEN', value: github, scope: [], ...at, purpose: 'GitHub connector' }, + { name: 'NOBODY_KEY', value: kept, scope: [], scopeSet: true, ...at }, + { name: 'WRITER_KEY', value: picked, scope: ['b2'], ...at }, @@ -965,12 +1048,92 @@ diff --git a/test/secrets.test.js b/test/secrets.test.js + t.after(bench.stop) + await bench.call('secret-set', { name: 'GITHUB_TOKEN', value }) + assert.deepEqual(bench.env('b2'), { DSH_SECRET_GITHUB_TOKEN: value }) -+ assert.deepEqual(bench.env('c3'), {}) ++ assert.deepEqual(bench.env('c3'), { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:unknown-turn' }) +}) + test('tool results and model requests lose the values', async (t) => { const bench = start() t.after(bench.stop) +@@ -302,3 +302,79 @@ test('tool results and model requests lose the values', async (t) => { + stream({ ...options, system: 'clean', messages: [] }, next) + assert.equal(passed.length, 2) + }) ++ ++// #651: a shell says which key was withheld and why, never the value. Before this, `resolveEnv` ++// returned `{}` and "the owner did not configure it" and "it was withheld" looked the same. ++const SHARED = { ...TEAM, grants: { b2: [{ grantee: '@bob.home', name: 'Bob' }] } } ++const saved = scope => [{ name: 'GITHUB_TOKEN', value: 'ghp_testvalue_123', scope, purpose: 'GitHub' }] ++// The seed is read asynchronously, so one state read settles it before the shell is asked. ++const seeded = async options => { const bench = start(options); await bench.view(); return bench } ++ ++test('#651: the owner’s own turn gets the key and says nothing about withholding', async (t) => { ++ const bench = await seeded({ secrets: saved('all') }) ++ t.after(bench.stop) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user' } } }) ++ const env = bench.env('b2') ++ assert.deepEqual(env, { DSH_SECRET_GITHUB_TOKEN: 'ghp_testvalue_123' }) ++ assert.equal(env.DSH_SECRETS_WITHHELD, undefined) ++}) ++ ++test('#651: someone else’s turn names the key and the reason, and carries no value', async (t) => { ++ const bench = await seeded({ secrets: saved('all') }) ++ t.after(bench.stop) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user', authorId: 'ann' } } }) ++ const env = bench.env('b2') ++ assert.deepEqual(env, { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:turn-not-owner' }) ++ assert.ok(!JSON.stringify(env).includes('ghp_testvalue_123'), 'no value anywhere in the shell env') ++}) ++ ++test('#651: a friend’s Bot says remote-bot, and a host with no key says nothing at all', async (t) => { ++ const friend = { ++ ...TEAM, ++ bots: { ...TEAM.bots, f9: { id: 'f9', sessionId: 'f9', name: 'Pal', role: '友', instructions: '', createdAt: 9, remote: { handle: '@pal.ann.home' } } }, ++ } ++ const bench = await seeded({ secrets: saved('all'), team: friend }) ++ t.after(bench.stop) ++ const env = bench.env('f9') ++ assert.deepEqual(env, { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:remote-bot' }) ++ assert.ok(!JSON.stringify(env).includes('ghp_testvalue_123')) ++ ++ // Nothing saved: the same `{}` as before, and the tool says so in those words (#651). ++ const empty = start() ++ t.after(empty.stop) ++ assert.deepEqual(empty.env('b2'), {}) ++ const listed = await empty.tool('list_secrets', {}, 'b2') ++ assert.match(listed.text, /No keys are saved yet/) ++ assert.ok(!listed.text.includes('withheld')) ++}) ++ ++test('#651: a shared Bot’s scheduled turn withholds scope "all" and keeps it for the Bot it names', async (t) => { ++ const sharedAll = await seeded({ secrets: saved('all'), team: SHARED }) ++ t.after(sharedAll.stop) ++ sharedAll.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) ++ sharedAll.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'schedule' } } }) ++ assert.deepEqual(sharedAll.env('b2'), { DSH_SECRETS_WITHHELD: 'GITHUB_TOKEN:shared-bot-scheduled' }) ++ ++ // The owner said which Bot may use this key, so that Bot's own scheduled turn still has it. ++ const sharedThis = await seeded({ secrets: saved(['b2']), team: SHARED }) ++ t.after(sharedThis.stop) ++ sharedThis.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) ++ sharedThis.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'schedule' } } }) ++ assert.deepEqual(sharedThis.env('b2'), { DSH_SECRET_GITHUB_TOKEN: 'ghp_testvalue_123' }) ++}) ++ ++test('#651: list_secrets and request_secret give the same reason, and never a value', async (t) => { ++ const bench = await seeded({ secrets: saved('all') }) ++ t.after(bench.stop) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'turn/start', data: { turn: 1 } }) ++ bench.ctx.emitArgs('session/event', { id: 'b2' }, { type: 'user/message', data: { content: [], source: { kind: 'user', authorId: 'ann' } } }) ++ const listed = await bench.tool('list_secrets', {}, 'b2') ++ assert.match(listed.text, /GITHUB_TOKEN/) ++ assert.match(listed.text, /withheld this turn: turn-not-owner/) ++ assert.ok(!listed.text.includes('ghp_testvalue_123')) ++ const asked = await bench.tool('request_secret', { name: 'GITHUB_TOKEN', purpose: 'GitHub' }, 'b2') ++ assert.match(asked.text, /withheld this turn: turn-not-owner/) ++ assert.ok(!asked.text.includes('ghp_testvalue_123')) ++}) diff --git a/test/store.test.js b/test/store.test.js --- a/test/store.test.js +++ b/test/store.test.js diff --git a/patches/ds-bot/files/src/host/turn-origin.js b/patches/ds-bot/files/src/host/turn-origin.js index 425b48981..52e35c748 100644 --- a/patches/ds-bot/files/src/host/turn-origin.js +++ b/patches/ds-bot/files/src/host/turn-origin.js @@ -105,6 +105,13 @@ export function combine(a, b) { return a === SCHEDULED || b === SCHEDULED ? SCHEDULED : OWNER } +// The verdict the running turn recorded, before `settled` narrows it. `scheduled` is the one a caller +// cannot get anywhere else: on a shared Bot it is 'other' as a permission, and still 'scheduled' as +// a fact, which is what a caller needs to say why a turn is not the owner's (#651). +export function turnOrigin(rt, sessionId, entry = rt.live?.get(sessionId)) { + return entry?.turn === undefined ? undefined : entry.origin +} + // The verdict that holds right now for the turn a Session is running: 'owner', 'other', or // undefined when the Session runs no turn or no message of it carried a verdict. `entry` is the // Session's `rt.live` record; the checks here read the team as it is at this moment, so a person