From 88a12929fa0f6b66d7f0fa295fbac49f8f8bd597 Mon Sep 17 00:00:00 2001 From: orveth Date: Wed, 27 May 2026 11:06:16 -0700 Subject: [PATCH 1/2] Implement per-agent environment isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the claude-code harness with declarative library option families and per-agent whitelists per docs/per-agent-environment-design.md (spec on per-agent-isolation-design / PR #12). What lands: 1. Three library option families: - services.forge.skills. { source; description; } - services.forge.mcpServers. { command; args; env; } - services.forge.plugins. { source; } 2. Library installation in modules/harnesses/claude-code.nix: - skills -> /etc/forge/skill-library// via environment.etc - plugins -> /etc/forge/plugin-library// via environment.etc - mcpServers stay in-config (referenced into per-agent .mcp.json) - Eval-time assertions for library uniqueness. 3. Per-agent whitelist + permissions schema on services.forge.agents.: - skills, mcpServers, plugins, allowedTools (listOf str, default []) - permissions { allow; deny; skipPrompts = true; } - Cross-cutting assertions: every reference must resolve in the library. 4. Per-agent CWD assembled on every start: - CLAUDE.md (from role) — already there - .claude/skills/ symlinks to /etc/forge/skill-library/ - .claude/settings.json from permissions.{allow,deny} - .claude/skill-catalog.md from declared skills' descriptions - .mcp.json from declared mcpServers (always written, even empty) - .env (discord token) — already there 5. New exec recipe per spec section "The exec recipe": - --bare dropped (breaks OAuth) - --strict-mcp-config dropped (strips plugin-bundled MCPs) - --mcp-config, --plugin-dir-per-plugin, --settings, --setting-sources project, --append-system-prompt-file, --allowedTools (only when non-empty) - Old --channels conditional removed; the discord plugin now flows through services.forge.plugins. 6. Demo agent in deployments/agicash-team-forge/configuration.nix updated to the new shape (kept commented-out until operator runs the sops bootstrap). Deferred per spec "Non-goals": - nix develop .#agent. devShell output - disallowedTools beyond schema - Per-agent Linux user - Plugin/comms split - Hooks family - Library namespacing - harness portability fields Validation run before pushing: - nix flake check passes - nix eval .#nixosConfigurations.agicash-team-forge.config.system.build.toplevel.drvPath resolves to a valid derivation - Eval test with declared skill/plugin/mcp confirms environment.etc entries resolve to store paths - Generated wrapper passes bash -n; empty-whitelist agent still evaluates and produces a valid script - Negative test: misspelled skill reference fires the assertion with the expected message Co-Authored-By: Claude Opus 4.7 (1M context) --- .../agicash-team-forge/configuration.nix | 51 ++++- modules/agents.nix | 175 ++++++++++++++-- modules/default.nix | 3 + modules/harnesses/claude-code.nix | 192 +++++++++++++++--- modules/mcp-servers.nix | 73 +++++++ modules/plugins.nix | 52 +++++ modules/skills.nix | 61 ++++++ 7 files changed, 568 insertions(+), 39 deletions(-) create mode 100644 modules/mcp-servers.nix create mode 100644 modules/plugins.nix create mode 100644 modules/skills.nix diff --git a/deployments/agicash-team-forge/configuration.nix b/deployments/agicash-team-forge/configuration.nix index c64d31e..16abb36 100644 --- a/deployments/agicash-team-forge/configuration.nix +++ b/deployments/agicash-team-forge/configuration.nix @@ -69,12 +69,17 @@ # 1. Follow docs/secrets-bootstrap.md to generate an age key, # register it in .sops.yaml, and create the encrypted # secrets.yaml file containing `team-bot-token`. - # 2. Uncomment the block below: it declares the secret, the - # Discord bot that consumes it, and the first agent that uses - # the bot. + # 2. Uncomment the blocks below: they declare the secret, the + # Discord bot that consumes it, the shared library (skills, + # MCP servers, plugins) the agent draws from, and finally the + # first agent itself with its per-agent whitelist. # 3. Redeploy. systemd starts `forge-agent-coordinator` on boot; # the agent joins Discord and responds to @mentions. # + # The `source` paths below are placeholders pending the migration of + # the v1 skills + dev MCP servers into this repository. See + # docs/per-agent-environment-design.md for the design contract. + # # sops.secrets."team-bot-token" = { # owner = "gudnuf"; # }; @@ -83,6 +88,30 @@ # tokenFile = config.sops.secrets."team-bot-token".path; # }; # + # # --- Shared library ---------------------------------------------------- + # # Skills, MCP servers, and plugins declared once at the top level; each + # # agent references them by name. Names must be unique across each family. + # + # services.forge.skills.discord-tools = { + # source = ../../skills/discord-tools; + # description = "Discord channel/thread/pin ops via REST"; + # }; + # + # services.forge.mcpServers.mercury = { + # command = "bun"; + # args = [ "run" "/srv/forge/plugins/mercury/server.ts" ]; + # env = { MERCURY_DB = "/var/lib/mercury/mercury.db"; }; + # }; + # + # services.forge.plugins.discord = { + # # Real claude-code plugin: directory containing + # # `.claude-plugin/plugin.json` plus the plugin's own commands, + # # MCP servers, etc. Loaded into the agent via --plugin-dir. + # source = ../../plugins/discord; + # }; + # + # # --- First agent ------------------------------------------------------- + # # services.forge.agents.coordinator = { # role = '' # agicash team coordinator — the on-call agent in the team @@ -91,6 +120,22 @@ # ''; # runAs = "gudnuf"; # discordBot = "team"; + # + # skills = [ "discord-tools" ]; + # mcpServers = [ "mercury" ]; + # plugins = [ "discord" ]; + # + # allowedTools = [ + # "Bash(git *)" + # "Edit" + # "Read" + # "mcp__mercury__send" + # "mcp__plugin_discord_discord__reply" + # ]; + # + # # permissions schema is in place for the future scoped-policy work; + # # today the harness still passes --dangerously-skip-permissions, so + # # the allow/deny lists are advisory. # }; # --- Nix GC --- diff --git a/modules/agents.nix b/modules/agents.nix index 76a72b7..515567a 100644 --- a/modules/agents.nix +++ b/modules/agents.nix @@ -62,6 +62,115 @@ in ''; example = "gudnuf"; }; + + # --- Per-agent isolation whitelist (see docs/per-agent-environment-design.md) --- + # Each list references shared library entries by name. The harness + # assembles the per-agent CWD from these whitelists on every start. + + skills = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Names of skills (from services.forge.skills) this agent has + access to. Each declared skill is symlinked into the agent's + per-CWD `.claude/skills/` and listed in the agent's + skill-catalog (appended to its system prompt). + + Skills not in this list are not invocable and not visible to + the agent's planner. + ''; + example = lib.literalExpression ''[ "discord-tools" "verify" ]''; + }; + + mcpServers = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Names of MCP servers (from services.forge.mcpServers) this + agent has access to. Each declared server is written into the + agent's per-CWD `.mcp.json`; servers not in this list are not + launched and their tools never appear in the model's surface. + ''; + example = lib.literalExpression ''[ "playwright" "mercury" ]''; + }; + + plugins = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Names of claude-code plugins (from services.forge.plugins) + this agent has access to. The harness adds a `--plugin-dir` + flag pointing at /etc/forge/plugin-library// for each + entry; the plugin's bundled MCP servers register with the + `mcp__plugin____` tool prefix. + ''; + example = lib.literalExpression ''[ "discord" ]''; + }; + + allowedTools = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Tool surface this agent is told it has, in claude-code's + --allowedTools syntax (e.g. "Bash(git *)", "Edit", "Read", + "mcp__plugin_discord_discord__reply"). Controls which tools + the model knows about and emits in tool-call grammar — distinct + from permission gating, which today is bypassed via + --dangerously-skip-permissions. + + Empty list means no --allowedTools flag is passed; the model + sees the harness defaults plus any plugin/MCP-contributed tools. + ''; + example = lib.literalExpression '' + [ "Bash(git *)" "Edit" "Read" "mcp__mercury__send" ] + ''; + }; + + permissions = lib.mkOption { + type = lib.types.submodule { + options = { + allow = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Tool-call patterns the agent is allowed to invoke without + prompting. Written to the per-agent .claude/settings.json. + Today the harness passes --dangerously-skip-permissions, + so this list is advisory; the schema seam exists for the + scoped-permissions follow-up. + ''; + }; + + deny = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Tool-call patterns the agent must never invoke. Same + caveats as `allow` — schema seam pending the scoped- + permissions follow-up. + ''; + }; + + skipPrompts = lib.mkOption { + type = lib.types.bool; + default = true; + description = '' + Reserved for the scoped-permissions follow-up. Today the + harness always passes --dangerously-skip-permissions; this + flag has no effect yet but is declared so callers can + start setting it and the harness can flip behavior when + the policy work lands. + ''; + }; + }; + }; + default = { }; + description = '' + Per-agent permission policy. Schema only at this stage — see + docs/per-agent-environment-design.md "Non-goals" for the + scoped-permissions deferral. + ''; + }; }; }); default = { }; @@ -86,18 +195,60 @@ in config = lib.mkIf cfg.enable { # Cross-cutting validation: every agent's runAs must reference a - # declared forge user. Catches typos at evaluation time instead of - # at session-start time. - assertions = lib.mapAttrsToList - (name: agent: { - assertion = builtins.hasAttr agent.runAs cfg.users; - message = '' - services.forge.agents.${name}.runAs = "${agent.runAs}" but - "${agent.runAs}" is not declared in services.forge.users. - Declare the user first, or correct the runAs. - ''; - }) - cfg.agents; + # declared forge user, and every library reference (skills, mcpServers, + # plugins) must point to a declared library entry. Catches typos at + # evaluation time instead of at session-start time. + assertions = + let + runAsChecks = lib.mapAttrsToList + (name: agent: { + assertion = builtins.hasAttr agent.runAs cfg.users; + message = '' + services.forge.agents.${name}.runAs = "${agent.runAs}" but + "${agent.runAs}" is not declared in services.forge.users. + Declare the user first, or correct the runAs. + ''; + }) + cfg.agents; + + libraryRefChecks = lib.concatLists (lib.mapAttrsToList + (name: agent: + (map + (skill: { + assertion = builtins.hasAttr skill cfg.skills; + message = '' + services.forge.agents.${name}.skills references "${skill}" + but "${skill}" is not declared in services.forge.skills. + Declare the skill in the library first, or correct the + reference. + ''; + }) + agent.skills) + ++ (map + (server: { + assertion = builtins.hasAttr server cfg.mcpServers; + message = '' + services.forge.agents.${name}.mcpServers references + "${server}" but "${server}" is not declared in + services.forge.mcpServers. Declare the server in the + library first, or correct the reference. + ''; + }) + agent.mcpServers) + ++ (map + (plugin: { + assertion = builtins.hasAttr plugin cfg.plugins; + message = '' + services.forge.agents.${name}.plugins references + "${plugin}" but "${plugin}" is not declared in + services.forge.plugins. Declare the plugin in the + library first, or correct the reference. + ''; + }) + agent.plugins)) + cfg.agents); + in + runAsChecks ++ libraryRefChecks; # Implementation (wrapper scripts, systemd services, harness wiring) # lives in sibling modules — kept here to declarations only so the diff --git a/modules/default.nix b/modules/default.nix index 8a73b43..a41d6d4 100644 --- a/modules/default.nix +++ b/modules/default.nix @@ -6,6 +6,9 @@ ./discord.nix ./agents.nix ./secrets.nix + ./skills.nix + ./mcp-servers.nix + ./plugins.nix ./harnesses/claude-code.nix ]; diff --git a/modules/harnesses/claude-code.nix b/modules/harnesses/claude-code.nix index 5ff7898..90678b7 100644 --- a/modules/harnesses/claude-code.nix +++ b/modules/harnesses/claude-code.nix @@ -3,29 +3,32 @@ # claude-code harness: turns declared agents into running processes. # # For each services.forge.agents. with harness = "claude-code": -# 1. A wrapper script `forge-agent-` that: -# - Ensures the agent's state dir exists (~/.local/state/forge/agents/) -# - Writes CLAUDE.md from the declared role -# - If discordBot is set, reads the bot token from the secret file and -# wires a .env + DISCORD_STATE_DIR for the claude discord plugin -# - Execs claude-code through a shared tmux server (socket: "forge") -# for PTY + attachability. claude-code needs a TTY; tmux provides it -# and makes the running session attachable. -# 2. A system systemd service running under the runAs user, supervised -# with restart-on-failure and start-rate limit. -# 3. claude-code + tmux + jq installed system-wide so any operator can +# 1. The shared forge library is installed once into /etc/forge/: +# - skills → /etc/forge/skill-library// (from services.forge.skills) +# - plugins → /etc/forge/plugin-library// (from services.forge.plugins) +# MCP servers don't need an install path — they're transcribed by +# command + args directly into each agent's .mcp.json. +# 2. A wrapper script `forge-agent-` builds the agent's per-CWD +# environment on every start (CLAUDE.md, .claude/skills/ symlinks, +# .claude/settings.json, .claude/skill-catalog.md, .mcp.json, .env) +# then execs claude-code through a shared tmux server. +# 3. A systemd service runs each agent under its declared runAs user, +# supervised with restart-on-failure and start-rate limit. +# 4. claude-code + tmux + jq installed system-wide so any operator can # attach or invoke wrappers manually. # +# Per-agent isolation (see docs/per-agent-environment-design.md): +# The wrapper assembles the agent's CWD from the declared whitelist — +# only the skills/plugins/MCP servers the agent has opted into appear. +# The library at /etc/forge/ is shared; the symlinks/files inside each +# per-agent CWD are not. +# # Operator cheat sheet (on the box): # systemctl start forge-agent- # start # systemctl status forge-agent- # state # journalctl -u forge-agent- -f # logs # sudo -u tmux -L forge attach -t agent- # attach # sudo -u tmux -L forge ls # list all agents on this user -# -# The harness lives in a per-harness module (modules/harnesses/.nix) -# so swapping or adding harnesses (codex, custom processes) doesn't touch -# the agent schema or any other module. let cfg = config.services.forge; @@ -33,9 +36,13 @@ let claudeAgents = lib.filterAttrs (_: a: a.harness == "claude-code") cfg.agents; - # Build CLAUDE.md as a build-time file so the role text doesn't have to - # be embedded (and escaped) in the wrapper shell. The wrapper copies it - # into the agent's state dir on every start (idempotent overwrite). + # --- Shared library install paths (referenced by both etc/ and wrappers) --- + skillLibraryDir = "/etc/forge/skill-library"; + pluginLibraryDir = "/etc/forge/plugin-library"; + + # --- Per-agent generated files (built at evaluation time, copied at start) --- + + # CLAUDE.md from declared role. mkClaudeMd = name: agent: pkgs.writeText "CLAUDE-${name}.md" '' # ${name} @@ -43,12 +50,79 @@ let ${agent.role} ''; + # .mcp.json — only declared MCP servers, in claude-code's native shape. + # ALWAYS write the file (even with an empty mcpServers map) because + # --mcp-config errors on missing files. + mkMcpJson = name: agent: + let + selected = lib.listToAttrs (map + (server: lib.nameValuePair server ( + let s = cfg.mcpServers.${server}; in + { + inherit (s) command args; + } // (lib.optionalAttrs (s.env != { }) { inherit (s) env; }) + )) + agent.mcpServers); + in + pkgs.writeText "mcp-${name}.json" (builtins.toJSON { + mcpServers = selected; + }); + + # .claude/settings.json — permission allow/deny carried from the schema. + # Actual permission gating today is bypassed by --dangerously-skip-permissions; + # the file exists so the seam is in place for the scoped-permissions follow-up. + mkSettingsJson = name: agent: + pkgs.writeText "settings-${name}.json" (builtins.toJSON { + permissions = { + allow = agent.permissions.allow; + deny = agent.permissions.deny; + }; + }); + + # .claude/skill-catalog.md — one-line description per declared skill, + # appended to the agent's system prompt so it knows what's available + # without us loading every skill body into context. + mkSkillCatalog = name: agent: + let + body = + if agent.skills == [ ] + then "" + else lib.concatMapStrings + (skill: "- /${skill} — ${cfg.skills.${skill}.description}\n") + agent.skills; + in + pkgs.writeText "skill-catalog-${name}.md" '' + # Available skills + + ${body}''; + + # Wrapper script. Builds the per-agent CWD on every start, then execs + # claude-code inside tmux. mkWrapper = name: agent: let claudeMd = mkClaudeMd name agent; + mcpJson = mkMcpJson name agent; + settingsJson = mkSettingsJson name agent; + skillCatalog = mkSkillCatalog name agent; + hasDiscord = agent.discordBot != null; tokenFile = if hasDiscord then cfg.discord.bots.${agent.discordBot}.tokenFile else ""; + + # --plugin-dir flags — one per declared plugin name. The harness + # points at /etc/forge/plugin-library// which environment.etc + # populates from the declared `source`. + pluginDirArgs = lib.concatMapStringsSep " " + (plugin: ''--plugin-dir "${pluginLibraryDir}/${plugin}"'') + agent.plugins; + + # --allowedTools — single comma-separated arg. Only emitted when + # the list is non-empty; an empty --allowedTools "" would strip the + # built-in tool surface (see spec "corrections from technical review"). + allowedToolsFlag = + if agent.allowedTools == [ ] + then "" + else ''--allowedTools "${lib.concatStringsSep "," agent.allowedTools}"''; in pkgs.writeShellApplication { name = "forge-agent-${name}"; @@ -57,11 +131,27 @@ let set -euo pipefail STATE_DIR="$HOME/.local/state/forge/agents/${name}" - mkdir -p "$STATE_DIR" - # Refresh CLAUDE.md from the declared role on every start. Treat the - # module-declared role as source of truth. - cp -f "${claudeMd}" "$STATE_DIR/CLAUDE.md" + # Per-agent skill symlinks live under .claude/skills/. Wipe the + # whole subtree on every start so removing a skill from the + # config actually removes the symlink from the agent's view; then + # recreate from scratch with the current whitelist. + rm -rf "$STATE_DIR/.claude/skills" + mkdir -p "$STATE_DIR/.claude/skills" + ${lib.concatMapStrings + (skill: '' + ln -sf "${skillLibraryDir}/${skill}" "$STATE_DIR/.claude/skills/${skill}" + '') + agent.skills} + + # Refresh per-agent generated files from the declaration. All are + # sourced from the Nix store (immutable) and copied into the + # agent's writable CWD so claude-code can read them without + # special-casing store paths. + cp -f "${claudeMd}" "$STATE_DIR/CLAUDE.md" + cp -f "${mcpJson}" "$STATE_DIR/.mcp.json" + cp -f "${settingsJson}" "$STATE_DIR/.claude/settings.json" + cp -f "${skillCatalog}" "$STATE_DIR/.claude/skill-catalog.md" ${lib.optionalString hasDiscord '' # Plumb the discord plugin: read the bot token from the secret @@ -81,13 +171,22 @@ let # claude-code is interactive — it needs a PTY. tmux provides the # PTY and makes the session attachable. Shared "forge" socket so # `tmux -L forge ls` enumerates every agent on this user. + # + # NOTE: --bare is intentionally NOT used (it disables OAuth/keychain + # reads, breaking subscription auth). --strict-mcp-config is also + # NOT used (it strips plugin-bundled MCP servers). See spec for the + # full justification of each flag. exec tmux -L forge new-session -A -s "agent-${name}" \ claude \ --model "${agent.model}" \ --effort max \ + --mcp-config .mcp.json \ + ${pluginDirArgs} \ + --settings .claude/settings.json \ + --setting-sources project \ + --append-system-prompt-file .claude/skill-catalog.md \ --dangerously-skip-permissions \ - ${lib.optionalString hasDiscord - ''--channels "plugin:discord@claude-plugins-official"''} + ${allowedToolsFlag} ''; }; @@ -108,6 +207,20 @@ let }; }; + # /etc/forge/skill-library// from services.forge.skills. + skillEtc = lib.mapAttrs' + (skill: s: lib.nameValuePair + "forge/skill-library/${skill}" + { source = s.source; }) + cfg.skills; + + # /etc/forge/plugin-library// from services.forge.plugins. + pluginEtc = lib.mapAttrs' + (plugin: p: lib.nameValuePair + "forge/plugin-library/${plugin}" + { source = p.source; }) + cfg.plugins; + in { config = lib.mkIf cfg.enable (lib.mkMerge [ @@ -115,6 +228,37 @@ in # have them whether or not any agent is declared yet. { environment.systemPackages = [ claudeCode pkgs.tmux ]; + + # Shared library installation. Attrset uniqueness is enforced at + # parse time, so duplicate library names can't reach this point — + # the defensive assertion below covers the same ground explicitly + # for anyone reading the eval output. + environment.etc = skillEtc // pluginEtc; + + # Eval-time sanity assertion. Nix attrsets enforce uniqueness on + # their own, but a named assertion produces a better error message + # if someone tries to merge configs with overlapping library entries + # via lib.mkMerge. + assertions = [ + { + assertion = + (lib.length (lib.attrNames cfg.skills)) + == (lib.length (lib.unique (lib.attrNames cfg.skills))); + message = "services.forge.skills has duplicate entries (this should be impossible — file a bug)."; + } + { + assertion = + (lib.length (lib.attrNames cfg.plugins)) + == (lib.length (lib.unique (lib.attrNames cfg.plugins))); + message = "services.forge.plugins has duplicate entries (this should be impossible — file a bug)."; + } + { + assertion = + (lib.length (lib.attrNames cfg.mcpServers)) + == (lib.length (lib.unique (lib.attrNames cfg.mcpServers))); + message = "services.forge.mcpServers has duplicate entries (this should be impossible — file a bug)."; + } + ]; } # Per-agent: systemd service + wrapper installed in systemPackages so diff --git a/modules/mcp-servers.nix b/modules/mcp-servers.nix new file mode 100644 index 0000000..acc53ef --- /dev/null +++ b/modules/mcp-servers.nix @@ -0,0 +1,73 @@ +{ config, lib, ... }: + +# MCP server library — declared once, referenced by name from agents. +# +# An MCP server is a local stdio process (Bun/Node/Python/binary) that +# claude-code launches via the `mcpServers` block of the per-agent +# .mcp.json file. Each declared server here corresponds to one entry in +# that JSON file when an agent whitelists it. +# +# These are NOT claude-code plugins (which have their own .claude-plugin/ +# manifest and ship bundled MCP servers under ${CLAUDE_PLUGIN_ROOT}). +# v1's `--dangerously-load-development-channels server:` mechanism +# maps to entries here in v2. +# +# This module declares the option family only. The harness reads +# `services.forge.mcpServers.` for each name in an agent's +# `mcpServers` list and writes the entries into the per-agent .mcp.json. + +{ + options.services.forge.mcpServers = lib.mkOption { + type = lib.types.attrsOf (lib.types.submodule { + options = { + command = lib.mkOption { + type = lib.types.str; + description = '' + Executable that launches this MCP server. Resolved against + the harness PATH at runtime — use absolute paths (e.g. + "''${pkgs.bun}/bin/bun") for hermetic invocation. + ''; + example = "bun"; + }; + + args = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + Arguments passed to the command. Same shape as claude-code's + native .mcp.json `args` field. + ''; + example = lib.literalExpression ''[ "run" "/srv/forge/plugins/mercury/server.ts" ]''; + }; + + env = lib.mkOption { + type = lib.types.attrsOf lib.types.str; + default = { }; + description = '' + Extra environment variables passed to the MCP server process. + Merged with the harness's own environment. + ''; + example = lib.literalExpression ''{ MERCURY_DB = "/var/lib/mercury/mercury.db"; }''; + }; + }; + }); + default = { }; + description = '' + Forge MCP-server library. Each entry is a local MCP server that + agents can opt into via their `mcpServers` field. Entries are + transcribed into the per-agent .mcp.json on harness start. + + Names must be unique across the library — the harness asserts + this at evaluation time. + ''; + example = lib.literalExpression '' + { + mercury = { + command = "bun"; + args = [ "run" "/srv/forge/plugins/mercury/server.ts" ]; + env = { MERCURY_DB = "/var/lib/mercury/mercury.db"; }; + }; + } + ''; + }; +} diff --git a/modules/plugins.nix b/modules/plugins.nix new file mode 100644 index 0000000..5d4c43b --- /dev/null +++ b/modules/plugins.nix @@ -0,0 +1,52 @@ +{ config, lib, ... }: + +# Plugin library — declared once, referenced by name from agents. +# +# A claude-code plugin is a directory containing a .claude-plugin/plugin.json +# manifest (and typically bundled commands, agents, hooks, MCP servers, etc.). +# The harness loads declared plugins via `--plugin-dir `; claude-code +# resolves ${CLAUDE_PLUGIN_ROOT} inside the plugin's bundled .mcp.json +# automatically when loaded this way. +# +# v1's `--channels plugin:@` mechanism (claude-plugins-official +# marketplace) maps to entries here in v2. Local plugins authored alongside +# forge live the same way — anything with a .claude-plugin/plugin.json works. +# +# This module declares the option family only. The harness copies each +# declared plugin into /etc/forge/plugin-library// and emits a +# `--plugin-dir /etc/forge/plugin-library/` flag per agent-whitelisted +# plugin. + +{ + options.services.forge.plugins = lib.mkOption { + type = lib.types.attrsOf (lib.types.submodule ({ name, ... }: { + options = { + source = lib.mkOption { + type = lib.types.path; + description = '' + Path to the plugin's root directory — the directory that + contains `.claude-plugin/plugin.json`. Installed (read-only) + into /etc/forge/plugin-library// by the harness. + ''; + example = lib.literalExpression "./plugins/discord"; + }; + }; + })); + default = { }; + description = '' + Forge plugin library. Each entry is a claude-code plugin (with a + `.claude-plugin/plugin.json` manifest) that agents can opt into + via their `plugins` field. + + Names must be unique across the library — the harness asserts + this at evaluation time. + ''; + example = lib.literalExpression '' + { + discord = { + source = ./plugins/discord; + }; + } + ''; + }; +} diff --git a/modules/skills.nix b/modules/skills.nix new file mode 100644 index 0000000..602e913 --- /dev/null +++ b/modules/skills.nix @@ -0,0 +1,61 @@ +{ config, lib, ... }: + +# Skill library — declared once, referenced by name from agents. +# +# Each entry under `services.forge.skills.` describes a claude-code +# skill: a directory containing a SKILL.md (and any supporting files) +# that the harness installs into the shared library at +# /etc/forge/skill-library//. +# +# Agents declare which skills they want via `services.forge.agents..skills` +# (defined in agents.nix). The harness then symlinks the agent's whitelist +# from the shared library into the agent's per-CWD `.claude/skills/` so the +# agent only sees its declared subset. +# +# This module declares the option family only. Installation (copy / symlink +# into /etc/forge/skill-library/) lives in modules/harnesses/claude-code.nix +# alongside the agent CWD assembly logic that consumes it. + +{ + options.services.forge.skills = lib.mkOption { + type = lib.types.attrsOf (lib.types.submodule ({ name, ... }: { + options = { + source = lib.mkOption { + type = lib.types.path; + description = '' + Path to the skill's root directory — typically contains a + SKILL.md plus any helper files. Installed (read-only) into + /etc/forge/skill-library// by the harness. + ''; + example = lib.literalExpression "./skills/discord-tools"; + }; + + description = lib.mkOption { + type = lib.types.str; + description = '' + One-line description of what this skill does. Surfaced in + the per-agent skill catalog (appended to the agent's system + prompt) so the agent knows when to invoke it. + ''; + example = "Discord channel/thread/pin ops via REST"; + }; + }; + })); + default = { }; + description = '' + Forge skill library. Each entry is a reusable claude-code skill + that agents can opt into via their `skills` field. + + Names must be unique across the library — the harness asserts this + at evaluation time. + ''; + example = lib.literalExpression '' + { + discord-tools = { + source = ./skills/discord-tools; + description = "Discord channel/thread/pin ops via REST"; + }; + } + ''; + }; +} From 8f7240907a972651763d235d7d271281db9379b4 Mon Sep 17 00:00:00 2001 From: orveth Date: Wed, 27 May 2026 11:27:44 -0700 Subject: [PATCH 2/2] Address dual-reviewer findings on per-agent isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Technical review surfaced one HIGH-priority security issue and one refuted spec claim. Design review surfaced three small consistency items. All amendments below; flake check passes. HIGH (technical A): Shell injection via interpolated Nix fields. Wrap shell-interpolated values with lib.escapeShellArg in: - plugin path construction (pluginDirArgs) - skill symlink targets + names - discord bot tokenFile read - agent.model in the exec line Threat model is "trusted Nix config author," but defense in depth matters when modules can be mkMerge'd from untrusted sources. REFUTED (technical B): --allowedTools has no observable effect on the agent's tool surface in claude-code 2.1.150 when --dangerously-skip-permissions is set. Empirical: with --allowedTools "Read,Bash(git *)", claude still reports Edit, Write, Glob, Grep, Agent, Task, etc. The actual tool-restriction flag is --tools (kept off — agents need full default tools today). Drop --allowedTools from the exec recipe. The agent.allowedTools schema field stays — it lands in settings.json for when scoped permissions are designed. DEAD CODE (technical D): Drop the three "library uniqueness" assertions in claude-code.nix. Attrset names are structurally unique in Nix; the substantive cross-reference assertions (agent.skills must reference a declared skill, etc.) live in modules/agents.nix and stay. CONSISTENCY (design 1, 2, 3): - Skill catalog separator: em-dash → colon (matches spec line 187) - Submodule shape: drop unused `({ name, ... }: ...)` from skills.nix and plugins.nix to match mcp-servers.nix + discord.nix - Demo `coordinator` exercises all three library families: add services.forge.mcpServers.playwright entry to the commented demo block and append "playwright" to coordinator.mcpServers Validation: `nix flake check` passes (with a temporary deploy-config.nix for eval; not committed). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../agicash-team-forge/configuration.nix | 14 +++- modules/harnesses/claude-code.nix | 77 +++++++------------ modules/plugins.nix | 4 +- modules/skills.nix | 4 +- 4 files changed, 45 insertions(+), 54 deletions(-) diff --git a/deployments/agicash-team-forge/configuration.nix b/deployments/agicash-team-forge/configuration.nix index 16abb36..324ca3e 100644 --- a/deployments/agicash-team-forge/configuration.nix +++ b/deployments/agicash-team-forge/configuration.nix @@ -103,6 +103,18 @@ # env = { MERCURY_DB = "/var/lib/mercury/mercury.db"; }; # }; # + # services.forge.mcpServers.playwright = { + # command = "npx"; + # args = [ + # "@playwright/mcp@latest" + # "--headless" + # "--executable-path" "/run/current-system/sw/bin/chromium" + # "--viewport-size" "390x844" + # "--output-dir" "/srv/forge/browser-output" + # "--save-session" + # ]; + # }; + # # services.forge.plugins.discord = { # # Real claude-code plugin: directory containing # # `.claude-plugin/plugin.json` plus the plugin's own commands, @@ -122,7 +134,7 @@ # discordBot = "team"; # # skills = [ "discord-tools" ]; - # mcpServers = [ "mercury" ]; + # mcpServers = [ "mercury" "playwright" ]; # plugins = [ "discord" ]; # # allowedTools = [ diff --git a/modules/harnesses/claude-code.nix b/modules/harnesses/claude-code.nix index 90678b7..df17da7 100644 --- a/modules/harnesses/claude-code.nix +++ b/modules/harnesses/claude-code.nix @@ -88,7 +88,7 @@ let if agent.skills == [ ] then "" else lib.concatMapStrings - (skill: "- /${skill} — ${cfg.skills.${skill}.description}\n") + (skill: "- /${skill}: ${cfg.skills.${skill}.description}\n") agent.skills; in pkgs.writeText "skill-catalog-${name}.md" '' @@ -109,20 +109,13 @@ let tokenFile = if hasDiscord then cfg.discord.bots.${agent.discordBot}.tokenFile else ""; - # --plugin-dir flags — one per declared plugin name. The harness - # points at /etc/forge/plugin-library// which environment.etc - # populates from the declared `source`. + # --plugin-dir flags — one per declared plugin name. Each arg is + # shell-escaped defensively even though plugin names come from the + # Nix config (defense-in-depth against future mkMerge from untrusted + # sources). pluginDirArgs = lib.concatMapStringsSep " " - (plugin: ''--plugin-dir "${pluginLibraryDir}/${plugin}"'') + (plugin: "--plugin-dir ${lib.escapeShellArg "${pluginLibraryDir}/${plugin}"}") agent.plugins; - - # --allowedTools — single comma-separated arg. Only emitted when - # the list is non-empty; an empty --allowedTools "" would strip the - # built-in tool surface (see spec "corrections from technical review"). - allowedToolsFlag = - if agent.allowedTools == [ ] - then "" - else ''--allowedTools "${lib.concatStringsSep "," agent.allowedTools}"''; in pkgs.writeShellApplication { name = "forge-agent-${name}"; @@ -135,12 +128,14 @@ let # Per-agent skill symlinks live under .claude/skills/. Wipe the # whole subtree on every start so removing a skill from the # config actually removes the symlink from the agent's view; then - # recreate from scratch with the current whitelist. + # recreate from scratch with the current whitelist. Skill names + # are shell-escaped defensively even though they come from the + # Nix config. rm -rf "$STATE_DIR/.claude/skills" mkdir -p "$STATE_DIR/.claude/skills" ${lib.concatMapStrings (skill: '' - ln -sf "${skillLibraryDir}/${skill}" "$STATE_DIR/.claude/skills/${skill}" + ln -sf ${lib.escapeShellArg "${skillLibraryDir}/${skill}"} ${lib.escapeShellArg "$STATE_DIR/.claude/skills/${skill}"} '') agent.skills} @@ -157,8 +152,9 @@ let # Plumb the discord plugin: read the bot token from the secret # file and write a per-agent .env. The claude discord plugin # consumes DISCORD_BOT_TOKEN from $DISCORD_STATE_DIR/.env. - if [ -f "${tokenFile}" ]; then - printf 'DISCORD_BOT_TOKEN=%s\n' "$(cat "${tokenFile}")" > "$STATE_DIR/.env" + # tokenFile is shell-escaped defensively. + if [ -f ${lib.escapeShellArg tokenFile} ]; then + printf 'DISCORD_BOT_TOKEN=%s\n' "$(cat ${lib.escapeShellArg tokenFile})" > "$STATE_DIR/.env" chmod 600 "$STATE_DIR/.env" else echo "WARN: discord bot token file ${tokenFile} not found; starting without discord" >&2 @@ -176,17 +172,24 @@ let # reads, breaking subscription auth). --strict-mcp-config is also # NOT used (it strips plugin-bundled MCP servers). See spec for the # full justification of each flag. + # + # NOTE on --allowedTools: empirical testing of claude-code 2.1.150 + # shows --allowedTools has no observable effect on the agent's tool + # surface when --dangerously-skip-permissions is set. The actual + # tool-surface-restriction flag is --tools (kept off here so agents + # have full default tools). agent.allowedTools is preserved as + # schema (it lands in settings.json) for when the scoped-permissions + # follow-up turns this back on. exec tmux -L forge new-session -A -s "agent-${name}" \ claude \ - --model "${agent.model}" \ + --model ${lib.escapeShellArg agent.model} \ --effort max \ --mcp-config .mcp.json \ ${pluginDirArgs} \ --settings .claude/settings.json \ --setting-sources project \ --append-system-prompt-file .claude/skill-catalog.md \ - --dangerously-skip-permissions \ - ${allowedToolsFlag} + --dangerously-skip-permissions ''; }; @@ -229,36 +232,12 @@ in { environment.systemPackages = [ claudeCode pkgs.tmux ]; - # Shared library installation. Attrset uniqueness is enforced at - # parse time, so duplicate library names can't reach this point — - # the defensive assertion below covers the same ground explicitly - # for anyone reading the eval output. + # Shared library installation. Attrset uniqueness is structurally + # enforced — Nix's module system already errors on overlapping + # definitions via lib.mkMerge — so no defensive assertion needed + # here. The substantive cross-reference assertions (agent.skills + # must reference declared skills, etc.) live in modules/agents.nix. environment.etc = skillEtc // pluginEtc; - - # Eval-time sanity assertion. Nix attrsets enforce uniqueness on - # their own, but a named assertion produces a better error message - # if someone tries to merge configs with overlapping library entries - # via lib.mkMerge. - assertions = [ - { - assertion = - (lib.length (lib.attrNames cfg.skills)) - == (lib.length (lib.unique (lib.attrNames cfg.skills))); - message = "services.forge.skills has duplicate entries (this should be impossible — file a bug)."; - } - { - assertion = - (lib.length (lib.attrNames cfg.plugins)) - == (lib.length (lib.unique (lib.attrNames cfg.plugins))); - message = "services.forge.plugins has duplicate entries (this should be impossible — file a bug)."; - } - { - assertion = - (lib.length (lib.attrNames cfg.mcpServers)) - == (lib.length (lib.unique (lib.attrNames cfg.mcpServers))); - message = "services.forge.mcpServers has duplicate entries (this should be impossible — file a bug)."; - } - ]; } # Per-agent: systemd service + wrapper installed in systemPackages so diff --git a/modules/plugins.nix b/modules/plugins.nix index 5d4c43b..e08a80c 100644 --- a/modules/plugins.nix +++ b/modules/plugins.nix @@ -19,7 +19,7 @@ { options.services.forge.plugins = lib.mkOption { - type = lib.types.attrsOf (lib.types.submodule ({ name, ... }: { + type = lib.types.attrsOf (lib.types.submodule { options = { source = lib.mkOption { type = lib.types.path; @@ -31,7 +31,7 @@ example = lib.literalExpression "./plugins/discord"; }; }; - })); + }); default = { }; description = '' Forge plugin library. Each entry is a claude-code plugin (with a diff --git a/modules/skills.nix b/modules/skills.nix index 602e913..8100f74 100644 --- a/modules/skills.nix +++ b/modules/skills.nix @@ -18,7 +18,7 @@ { options.services.forge.skills = lib.mkOption { - type = lib.types.attrsOf (lib.types.submodule ({ name, ... }: { + type = lib.types.attrsOf (lib.types.submodule { options = { source = lib.mkOption { type = lib.types.path; @@ -40,7 +40,7 @@ example = "Discord channel/thread/pin ops via REST"; }; }; - })); + }); default = { }; description = '' Forge skill library. Each entry is a reusable claude-code skill