From 4276b53a8f1db252345c6c57885170adc7c067cd Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 4 Nov 2026 17:28:52 +0100 Subject: [PATCH] fix: keep credential files out of the file routes and Discord uploads (release review) The Discord bot token was readable through the attachment routes, because discord.json was not on the sensitive list, and so were webhook.json, custom-model-hosts.json and the generated custom-model configs. Config backups were the wider gap: a dated copy of users.json or settings.json matched none of the name-anchored patterns. All are blocked now. `codeman notify --file` relied on folder confinement alone, which holds nothing back when a tab's folder is the home directory, and it would also post a workspace .env. It now applies the attachment blocklist to the real path. Co-Authored-By: Claude Opus 5.5 --- docs/discord-bridge.md | 2 +- docs/security-architecture.md | 2 +- docs/wiki/Discord-Bridge.md | 2 +- src/web/routes/discord-routes.ts | 10 ++++++++-- src/web/sensitive-path.ts | 11 +++++++++++ test/routes/discord-routes.test.ts | 28 +++++++++++++++++++++++++++- test/sensitive-path.test.ts | 8 ++++++++ 7 files changed, 57 insertions(+), 6 deletions(-) diff --git a/docs/discord-bridge.md b/docs/discord-bridge.md index 161f533f..86210d3e 100644 --- a/docs/discord-bridge.md +++ b/docs/discord-bridge.md @@ -110,7 +110,7 @@ The text explains `codeman notify "message" --urgent`. `codeman` is on the PATH ## Files - **Received:** files you attach to a reply from an allowed user are saved as `/.codeman-discord/-` (mode 0600) and the one-line reply the CLI sees ends with `[attached: ]`. At most 5 files of 8 MB each; names are reduced to letters, digits, `.`, `_` and `-`; only Discord's own HTTPS addresses are fetched. Anything skipped is named in the thread. The folder ignores itself (it holds a `.gitignore` of `*`), so nothing in it is committed. Remote (ssh) tabs have no local folder, so no file is saved for them. -- **Sent:** `codeman notify "message" --file ` uploads a regular file of at most 8 MB. The real path (symlinks followed) must be inside the tab's working folder, which stops an upload of something outside it by mistake. The agent runs as your user, so this is a guard against accidents, not a sandbox. Remote tabs cannot send files. +- **Sent:** `codeman notify "message" --file ` uploads a regular file of at most 8 MB. The real path (symlinks followed) must be inside the tab's working folder, and a file on the attachment blocklist (`.env`, keys under `.ssh`, Codeman's own state) is refused even there, which stops an upload of something sensitive by mistake. The agent runs as your user, so this is a guard against accidents, not a sandbox. Remote tabs cannot send files. - A downloaded file is untrusted content: only allowed users can send one, but a CLI that opens it is exposed to anything written inside it. ## Security model diff --git a/docs/security-architecture.md b/docs/security-architecture.md index f25f843f..3234ea53 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -582,7 +582,7 @@ Opt-in and off by default: the server POSTs the Web Push events (permission prom - **A second server-side outbound channel through the web-tab egress guard (ยง10b).** Delivery goes through `webviewFetch`, so link-local and cloud-metadata targets are refused at save time and again on the RESOLVED address at connect time; redirects are not followed (`redirect: 'manual'`) and each send is bounded by a 5 s timeout. Loopback and RFC1918 stay allowed on purpose (a self-hosted ntfy is the point), so **Send test** works as a blind reachability probe (status, refused or timed out, never a response body) for whoever may call it. Web tabs already give that caller full LAN reach with bodies, so nothing new is exposed. - **The URL is a bearer secret** (anyone holding a Slack or Discord webhook URL can post as it). It lives in `~/.codeman/webhook.json` (0600, tmp+rename), is kept out of `settings.json` (which every logged-in user reads through `GET /api/settings`), is never returned (`GET /api/webhook` gives scheme + host only), and never appears in a log line, a delivery result or an error message. -- **Discord bridge** (`src/discord-bridge.ts`, [setup](discord-bridge.md)): the bot token lives in `~/.codeman/discord.json` (0600, tmp+rename) and is never returned, logged or put in an error. Inbound replies are typed into a session only from `allowedUserIds`, in the configured guild, inside a thread the bridge created; anything else is dropped unread. A reply becomes one printable line plus Enter, capped at 1000 characters and one per second per tab. Posts send `allowed_mentions` limited to the allowlisted ids, so agent output cannot ping a server. **Files:** an attachment is fetched only from Discord's own HTTPS hosts (no redirects followed), at most 5 per reply and 8 MB each, saved 0600 under `/.codeman-discord/` with the name reduced to `[A-Za-z0-9._-]`; a file a CLI sends must be a regular file whose real path (symlinks followed) lies inside its own tab's folder, which stops a file outside that folder being uploaded by mistake, and remote sessions send and receive none. A received file is untrusted content to whatever opens it. **What the checks do not stop:** the agent runs as the same OS user, so the path check prevents accidental uploads, not a determined agent that pastes a file's contents into the message. **What leaves the machine:** a permission or question alert sends the rendered screen (whatever the agent printed), and a finished turn or an error sends the CLI's whole last message, read from its transcript (up to 8 posts of 1,900 characters). `idle_prompt` is a warning, so with the default scope every Claude turn left unanswered for a minute is posted, and with **Which events** set to Everything every turn end is; treat the Discord server as holding that output. **Multi-user:** the bridge is one allowlist for the whole server, so an allowlisted Discord account can type into every user's sessions; only an admin can configure it. **Startup hint:** a fixed text (no user input) is added to Claude's system prompt; no other CLI is sent it. The bridge only dials out (REST and the gateway WebSocket), so it needs no inbound exposure. A Discord account on the allowlist can drive agents that run shell commands: it needs 2FA. +- **Discord bridge** (`src/discord-bridge.ts`, [setup](discord-bridge.md)): the bot token lives in `~/.codeman/discord.json` (0600, tmp+rename) and is never returned, logged or put in an error. Inbound replies are typed into a session only from `allowedUserIds`, in the configured guild, inside a thread the bridge created; anything else is dropped unread. A reply becomes one printable line plus Enter, capped at 1000 characters and one per second per tab. Posts send `allowed_mentions` limited to the allowlisted ids, so agent output cannot ping a server. **Files:** an attachment is fetched only from Discord's own HTTPS hosts (no redirects followed), at most 5 per reply and 8 MB each, saved 0600 under `/.codeman-discord/` with the name reduced to `[A-Za-z0-9._-]`; a file a CLI sends must be a regular file whose real path (symlinks followed) lies inside its own tab's folder and is not on the attachment blocklist (`isBlockedAttachmentPath`, since a tab's folder can be the home directory), which stops a sensitive file being uploaded by mistake, and remote sessions send and receive none. A received file is untrusted content to whatever opens it. **What the checks do not stop:** the agent runs as the same OS user, so the path check prevents accidental uploads, not a determined agent that pastes a file's contents into the message. **What leaves the machine:** a permission or question alert sends the rendered screen (whatever the agent printed), and a finished turn or an error sends the CLI's whole last message, read from its transcript (up to 8 posts of 1,900 characters). `idle_prompt` is a warning, so with the default scope every Claude turn left unanswered for a minute is posted, and with **Which events** set to Everything every turn end is; treat the Discord server as holding that output. **Multi-user:** the bridge is one allowlist for the whole server, so an allowlisted Discord account can type into every user's sessions; only an admin can configure it. **Startup hint:** a fixed text (no user input) is added to Claude's system prompt; no other CLI is sent it. The bridge only dials out (REST and the gateway WebSocket), so it needs no inbound exposure. A Discord account on the allowlist can drive agents that run shell commands: it needs 2FA. - **It carries session data to a third party.** Titles and bodies include session names, tool names and error text, all agent- or user-controlled, so Discord gets `allowed_mentions: { parse: [] }` and Slack's `& < >` are escaped: agent output cannot ping a channel. In multi-user mode all three routes are admin-only and the channel is instance-wide: it receives every user's session events, the same reach an admin's own Web Push has, which means non-admins' session details leave the box at the admin's choice. --- diff --git a/docs/wiki/Discord-Bridge.md b/docs/wiki/Discord-Bridge.md index e566889b..1694a04f 100644 --- a/docs/wiki/Discord-Bridge.md +++ b/docs/wiki/Discord-Bridge.md @@ -79,7 +79,7 @@ Discord for them; only the agent-initiated `codeman notify` habit is missing. `/.codeman-discord/-` and the line the agent sees ends with `[attached: ]`. At most 5 files of 8 MB each. The folder ignores itself, so nothing in it is committed. - **Agent to you:** `codeman notify "message" --file ` uploads a file of at most 8 MB that lies - inside the tab's folder. + inside the tab's folder. Credential files such as `.env` are refused. - Remote (ssh) tabs have no local folder, so they neither send nor receive files. - A file you send is untrusted content to the agent that opens it. diff --git a/src/web/routes/discord-routes.ts b/src/web/routes/discord-routes.ts index f77df9e1..2ab85511 100644 --- a/src/web/routes/discord-routes.ts +++ b/src/web/routes/discord-routes.ts @@ -17,6 +17,7 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'; import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js'; import { canAccessOwned, findSessionOrFail, getAuthUser, isAdmin, parseBody } from '../route-helpers.js'; import type { SessionPort } from '../ports/session-port.js'; +import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from '../../config/attachment-guard.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { describeUnknownPath, probePathKind } from '../../utils/bounded-path-probe.js'; import { DiscordNotifySchema, DiscordUpdateSchema } from '../schemas.js'; @@ -80,8 +81,10 @@ export function _resetNotifyBuckets(): void { /** * A file a session may send: a regular file, at most MAX_FILE_BYTES, whose REAL path (symlinks - * followed) lies inside the session's working folder. Anything else is refused, so a CLI cannot be - * talked into posting ~/.codeman/discord.json or an ssh key. Returns the bytes, or the reason. + * followed) lies inside the session's working folder and is not on the attachment blocklist. The + * blocklist matters as much as the folder: with a home directory as the session folder, confinement + * alone would let a CLI be talked into posting ~/.codeman/discord.json, an ssh key or a `.env`. + * Returns the bytes, or the reason. */ async function fileProblem( workingDir: string, @@ -103,6 +106,9 @@ async function fileProblem( const rel = relative(root, real); if (rel === '' || rel === '..' || rel.startsWith(`..${sep}`) || isAbsolute(rel)) return 'The file must be inside the session folder'; + if (isBlockedAttachmentPath(real, (await loadAttachmentGuardConfig()).blockedTrees)) { + return 'That file cannot be sent: it can hold credentials'; + } const info = await stat(real); if (!info.isFile()) return 'Not a regular file'; if (info.size > MAX_FILE_BYTES) return `The file is over ${MAX_FILE_BYTES / 1024 / 1024} MB`; diff --git a/src/web/sensitive-path.ts b/src/web/sensitive-path.ts index 9f5fef4c..0189dbe4 100644 --- a/src/web/sensitive-path.ts +++ b/src/web/sensitive-path.ts @@ -106,6 +106,17 @@ const SENSITIVE_PATTERNS: RegExp[] = [ // agent wrote, keys and .env contents included; written 0600 and kept out of // /api/search for the same reason as intents.json. /\/\.codeman[^/]*\/edit-history\//, + // Credentials kept outside settings.json: the Discord bot token, the webhook URL + // (a bearer secret), the custom endpoints' API keys, and the generated per-session + // configs, some of which carry that key inline. + /\/\.codeman[^/]*\/discord\.json$/, + /\/\.codeman[^/]*\/webhook\.json$/, + /\/\.codeman[^/]*\/custom-model-hosts\.json$/, + /\/\.codeman[^/]*\/custom-model-configs\//, + // Config backups are dated COPIES of the files above (users.json and settings.json + // included), which the name-anchored patterns do not match. A custom + // `configBackupDir` is outside what a pattern here can name. + /\/\.codeman[^/]*\/backups\//, ]; /** diff --git a/test/routes/discord-routes.test.ts b/test/routes/discord-routes.test.ts index 7ea15bc3..48da64e4 100644 --- a/test/routes/discord-routes.test.ts +++ b/test/routes/discord-routes.test.ts @@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { createRouteTestHarness } from './_route-test-utils.js'; import { _resetNotifyBuckets, registerDiscordRoutes } from '../../src/web/routes/discord-routes.js'; import { DiscordBridge, discordConfigPath, readDiscordConfig, type DiscordFetch } from '../../src/discord-bridge.js'; @@ -286,6 +286,32 @@ describe('discord-notify with a file', () => { bridge.stop(); }); + it.each([ + ['a .env file', () => ['.env', 'TOKEN=x']], + ['a private key under .ssh', () => ['.ssh/id_ed25519', 'key']], + ['Codeman state with the bot token', () => ['.codeman/discord.json', '{}']], + ])('refuses %s even inside the session folder, as with a home directory', async (_n, entry) => { + const [rel, body] = entry(); + mkdirSync(dirname(join(work, rel)), { recursive: true }); + writeFileSync(join(work, rel), body); + const { app, bridge } = await ready(); + const res = await post(app, join(work, rel)); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/credentials/); + expect(fetchImpl.mock.calls.some((c) => c[1].body instanceof FormData)).toBe(false); + bridge.stop(); + }); + + it('refuses a symlink inside the folder that points at a blocked file', async () => { + writeFileSync(join(work, '.env'), 'TOKEN=x'); + symlinkSync(join(work, '.env'), join(work, 'notes.txt')); + const { app, bridge } = await ready(); + const res = await post(app, join(work, 'notes.txt')); + expect(res.statusCode).toBe(400); + expect(fetchImpl.mock.calls.some((c) => c[1].body instanceof FormData)).toBe(false); + bridge.stop(); + }); + it('refuses to send when the session folder is not a directory it can probe', async () => { const { app, ctx, bridge } = await ready(); (ctx.sessions.get('test-session-1') as unknown as { workingDir: string }).workingDir = join(work, 'run.log'); diff --git a/test/sensitive-path.test.ts b/test/sensitive-path.test.ts index cf0e7c60..c24b26ac 100644 --- a/test/sensitive-path.test.ts +++ b/test/sensitive-path.test.ts @@ -93,6 +93,14 @@ describe('isSensitivePath', () => { // contents included): 0600 and kept out of /api/search like intents.json. ['codeman edit history store', `${HOME}/.codeman/edit-history/s1.json`], ['codeman edit history on a named instance', `${HOME}/.codeman-beta/edit-history/s1.json`], + // Credentials outside settings.json, and the backups that copy all of these. + ['codeman Discord bot token', `${HOME}/.codeman/discord.json`], + ['codeman Discord token on a named instance', `${HOME}/.codeman-beta/discord.json`], + ['codeman webhook URL', `${HOME}/.codeman/webhook.json`], + ['codeman custom endpoint API keys', `${HOME}/.codeman/custom-model-hosts.json`], + ['a generated custom model config', `${HOME}/.codeman/custom-model-configs/s1/opencode.json`], + ['a backed-up user list', `${HOME}/.codeman/backups/config/cfg-20261104-120000/users.json`], + ['a backed-up settings file', `${HOME}/.codeman/backups/config/cfg-20261104-120000/settings.json`], ]; it.each(blocked)('blocks the %s', (_label, path) => {