feat: decompose pct_create into atomic create + agent-driven install; add scoped destructive window
Some checks failed
ci / build-test (push) Has been cancelled
ci / docker-build (push) Has been cancelled

Closes the two remaining open points from the auto-continuation work.

1. Atomic pct_create (observability, the bigger of the two):
   pct_create used to bundle create + apt install + post_install script into
   one black-box multi-minute SSH call — the agent got back a single opaque
   success/fail with no way to see (or fix) which step actually broke.
   Removed the whole post-create provisioning block (and the now-dead
   provisionScript/sanitizePkgs helpers + their tests). pct_create is now
   create + start + register ONLY — fast, and its result is fed back to the
   agent via auto-continuation almost immediately. The agent installs
   packages and runs setup as its OWN sequence of `run` calls against the new
   lxc:<hostname>, observing each command's real output and able to diagnose
   and retry exactly the step that failed — the same recovery loop already
   proven for the general case, now applied to installs too, instead of
   requiring a separate black-box mechanism.
   - services/post_install removed from the pct_create params struct and
     from the MCP tool schema/SOUL.md docs.
   - SOUL.md: explains the new flow, moves the Docker CLI gotcha and DNS
     troubleshooting guidance to be steps the agent runs itself.

2. Scoped destructive window (targeted autonomy for recovery):
   Verified live in the previous session that a destructive recovery (a
   failed destroy needing stop-then-destroy on the same container) required
   TWO separate typed confirmations for what was clearly one recovery
   action. Added a narrow, TARGET-scoped 15-minute grant
   (destructive_window.agent:<id>.target:<slug> in autonomy_settings,
   shared key format across cmd/nomos and internal/mcp) that opens only
   after an EXPLICIT typed confirmation (never loose assent) or an explicit
   button-approval of a destructive step, and only ever covers further
   destructive commands against that SAME target. A different target always
   needs its own fresh confirmation — this narrows risk instead of loosening
   it globally, unlike broadening the general assent window to cover
   destructive actions would have.
   - cmd/nomos/store.go: openDestructiveWindow/destructiveWindowActive/
     executionTarget.
   - cmd/nomos/agent.go: opens the window when a typed confirmation grants a
     destructive chat-assent execution.
   - internal/mcp/server.go: `run` tool checks the window before gating a
     destructive command; auto-runs if active.
   - internal/httpapi/phase3.go: DecideApproval opens the same window when a
     destructive execution is approved via the button/API, for parity with
     the chat-assent path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-10 14:45:09 +02:00
parent 6f9998fa29
commit 2e922f6421
6 changed files with 208 additions and 150 deletions

View File

@@ -2,7 +2,6 @@ package httpapi
import (
"encoding/json"
"strings"
"testing"
)
@@ -114,44 +113,7 @@ func TestGatewayPreflightPassed(t *testing.T) {
}
}
func TestProvisionScript(t *testing.T) {
s := provisionScript([]string{"docker.io", "git"}, "echo hi > /root/x")
// Network/DNS gate must come before apt.
gate := strings.Index(s, "getent hosts")
apt := strings.Index(s, "apt-get update")
post := strings.Index(s, "echo hi > /root/x")
if gate < 0 || apt < 0 || post < 0 {
t.Fatalf("missing sections: gate=%d apt=%d post=%d\n%s", gate, apt, post, s)
}
if !(gate < apt && apt < post) {
t.Errorf("wrong ordering: gate=%d apt=%d post=%d", gate, apt, post)
}
if !strings.Contains(s, "nameserver 1.1.1.1") {
t.Error("missing DNS self-heal fallback")
}
if !strings.Contains(s, "docker.io git") {
t.Error("packages not joined into install line")
}
// No packages: no apt lines, but post_install and gate still present.
s2 := provisionScript(nil, "systemctl status foo")
if strings.Contains(s2, "apt-get install") {
t.Error("apt install should be absent when no packages requested")
}
if !strings.Contains(s2, "systemctl status foo") || !strings.Contains(s2, "getent hosts") {
t.Error("post_install or gate missing in no-package case")
}
}
func TestSanitizePkgs(t *testing.T) {
in := []string{"docker.io", "git", "rm -rf /", "curl;wget", "python3-pip", ""}
got := sanitizePkgs(in)
want := map[string]bool{"docker.io": true, "git": true, "python3-pip": true}
if len(got) != len(want) {
t.Fatalf("got %v want keys %v", got, want)
}
for _, g := range got {
if !want[g] {
t.Errorf("unexpected package survived sanitize: %q", g)
}
}
}
// provisionScript and sanitizePkgs were removed when pct_create was made
// atomic (create + start + register only) — installing packages and running
// setup scripts is now the agent's own job via follow-up `run` calls, which
// already has its own classifier/sanitization tests in internal/policy.

View File

@@ -292,8 +292,12 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID,
Mounts []string `json:"mounts"`
Nameserver string `json:"nameserver"`
Searchdomain string `json:"searchdomain"`
Services []string `json:"services"` // apt packages to install after create
PostInstall string `json:"post_install"` // shell run inside the container after create
// No services/post_install here anymore — pct_create is atomic
// (create + start + register only). Installing packages and
// running setup scripts is the agent's job via follow-up `run`
// calls against lxc:<hostname>, so each step is individually
// observable and recoverable instead of one opaque multi-minute
// black box. See the comment above the removed post-create block.
}
if err := json.Unmarshal([]byte(params), &cfg); err != nil {
slog.Error("httpapi: pct_create parse params", "error", err, "params", params)
@@ -475,21 +479,24 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID,
slog.Info("httpapi: pct_create running", "vmid", cfg.VMID, "hostname", cfg.Hostname, "cmd", createCmd)
output, err = sshExec(ctx, host, user, createCmd)
// Post-create provisioning: install apt packages and run a post_install
// script inside the fresh container, so a single approved pct_create
// yields a *working service*, not just an empty container. The script
// waits for real DNS/connectivity and self-heals the resolver first —
// a static-IP container with a dead nameserver otherwise fails apt with
// "Temporary failure resolving deb.debian.org" and installs nothing.
if err == nil && (len(cfg.Services) > 0 || cfg.PostInstall != "") {
script := provisionScript(sanitizePkgs(cfg.Services), cfg.PostInstall)
b64 := base64.StdEncoding.EncodeToString([]byte(script))
// sleep on the host so the container is up enough to accept pct exec.
cmd := fmt.Sprintf("sleep 4; pct exec %d -- bash -c 'echo %s | base64 -d | bash'", cfg.VMID, b64)
var provOut string
provOut, err = sshExec(ctx, host, user, cmd)
output = output + "\n--- post-install ---\n" + provOut
}
// pct_create is now DELIBERATELY ATOMIC: create + start + register,
// nothing else. It used to also run apt installs and a post_install
// script inline as one black-box multi-minute SSH call — the agent
// got back a single opaque success/fail for the whole thing with no
// way to see (or fix) which step actually broke. That's the opposite
// of what makes an agent able to recover from errors.
//
// Installing packages, running post_install, and verifying the
// service now happen as the agent's OWN follow-up `run` calls against
// the new lxc:<hostname> target — each one is synchronous (in an
// active assent window) or individually gated, so the agent observes
// every step's real output and can diagnose + retry the exact thing
// that failed instead of re-doing the whole container. See SOUL.md
// "After pct_create: you drive the install" and provisionScript's
// surviving role (DNS self-heal) is now something the agent invokes
// itself via `run`, not something baked into this handler.
//
// cfg.Services/cfg.PostInstall are intentionally no longer read here.
// On success, register the entity in the DB with proper relationships
if err == nil {
@@ -583,47 +590,6 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID,
"execution_id", execID, "action", action, "status", status, "duration_ms", durationMs)
}
// provisionScript builds the in-container bootstrap run after pct create. It
// (1) waits for DNS/connectivity and self-heals /etc/resolv.conf with a public
// resolver if the configured nameserver is dead, (2) installs apt packages with
// retries, (3) runs the operator's post_install. `set -e` after the network
// gate means any apt or post_install failure exits non-zero, so sshExec surfaces
// it and the execution is marked failed with the exact broken step in output.
func provisionScript(pkgs []string, postInstall string) string {
var b strings.Builder
b.WriteString("set -o pipefail\n")
// A fresh debian LXC has no locale set, which spams "Can't set locale"
// warnings and breaks some package post-install scripts. Pin C.UTF-8.
b.WriteString("export LANG=C.UTF-8 LC_ALL=C.UTF-8 DEBIAN_FRONTEND=noninteractive\n")
b.WriteString("probe=deb.debian.org\n")
b.WriteString("ok=0\n")
// `timeout 3` on every getent call is load-bearing, not cosmetic: when
// the network is truly unreachable (e.g. a wrong gateway), a plain
// `getent hosts` doesn't fail fast — it can hang far longer than the
// resolver's nominal timeout because packets are just dropped, not
// rejected. Without a hard per-attempt cap, this loop's "~90s" budget
// was fiction — one run hung 17+ minutes on a bad gateway before the Go
// side finally got a hard sshExec timeout to fall back on. Capping each
// attempt makes the wall-clock budget real.
b.WriteString("for i in $(seq 1 30); do if timeout 3 getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done\n")
// Self-heal: if the assigned resolver can't resolve, fall back to public DNS.
b.WriteString("if [ \"$ok\" != 1 ]; then printf 'nameserver 1.1.1.1\\nnameserver 8.8.8.8\\n' > /etc/resolv.conf; ")
b.WriteString("for i in $(seq 1 15); do if timeout 3 getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done; fi\n")
b.WriteString("if [ \"$ok\" != 1 ]; then echo 'ERROR: container has no DNS/connectivity after ~2min — check the LXC net0 gateway/IP are correct for this subnet'; exit 1; fi\n")
b.WriteString("set -e\n")
if len(pkgs) > 0 {
b.WriteString("export DEBIAN_FRONTEND=noninteractive\n")
b.WriteString("apt-get update -o Acquire::Retries=3 -qq\n")
b.WriteString("apt-get install -y -o Acquire::Retries=3 --no-install-recommends -qq " + strings.Join(pkgs, " ") + "\n")
}
if strings.TrimSpace(postInstall) != "" {
b.WriteString("# --- operator post_install ---\n")
b.WriteString(postInstall)
b.WriteString("\n")
}
return b.String()
}
// jsonErr builds a valid {"error": "..."} JSON payload for an execution's
// result column. Always use this instead of fmt.Sprintf'ing JSON by hand —
// error text and command output routinely contain quotes/backslashes that
@@ -682,29 +648,6 @@ func resolveTemplate(requested string, available []string) string {
return best
}
// sanitizePkgs drops anything that isn't a plausible apt package token, so a
// hallucinated package list can't inject shell into the install command.
func sanitizePkgs(pkgs []string) []string {
out := make([]string, 0, len(pkgs))
for _, p := range pkgs {
p = strings.TrimSpace(p)
if p == "" {
continue
}
ok := true
for _, r := range p {
if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '-' || r == '.' || r == '+') {
ok = false
break
}
}
if ok {
out = append(out, p)
}
}
return out
}
// ─── Checks ────────────────────────────────────────────────────────────
func (s *Server) ListChecks(ctx context.Context, req gen.ListChecksRequestObject) (gen.ListChecksResponseObject, error) {
@@ -1474,12 +1417,12 @@ func (s *Server) DecideApproval(ctx context.Context, req gen.DecideApprovalReque
// On approve: execute the linked gated command.
if status == "approved" {
var execID, targetID uuid.UUID
var actionStr, targetSlug string
var actionStr, targetSlug, riskClass string
err := tx.QueryRow(ctx, `
SELECT e.entity_id, e.target_entity_id, e.action
SELECT e.entity_id, e.target_entity_id, e.action, e.risk_class
FROM executions e
WHERE e.approval_id = $1 AND e.status = 'pending_approval'
LIMIT 1`, id).Scan(&execID, &targetID, &actionStr)
LIMIT 1`, id).Scan(&execID, &targetID, &actionStr, &riskClass)
if err == nil {
// Resolve target entity slug from targetID.
_ = tx.QueryRow(ctx, "SELECT slug FROM entities WHERE id = $1", targetID).Scan(&targetSlug)
@@ -1504,6 +1447,21 @@ func (s *Server) DecideApproval(ctx context.Context, req gen.DecideApprovalReque
expires := time.Now().Add(30 * time.Minute).UTC().Format(time.RFC3339)
_, _ = tx.Exec(ctx, `INSERT INTO autonomy_settings (key, value) VALUES ($1, $2)
ON CONFLICT (key) DO UPDATE SET value = $2`, "assent_window.agent:"+agentID.String(), expires)
// Approving a DESTRUCTIVE step via the button is exactly as
// explicit as a typed "I confirm" — the operator affirmatively
// clicked Approve on a card that said DESTRUCTIVE. Open the
// same short, target-scoped destructive window chat-assent's
// typed-confirm path opens, for parity: a multi-step
// destructive recovery (stop, then destroy) shouldn't need a
// fresh confirmation per click any more than it needs one per
// typed phrase.
if riskClass == "destructive" && targetSlug != "" {
dExpires := time.Now().Add(15 * time.Minute).UTC().Format(time.RFC3339)
_, _ = tx.Exec(ctx, `INSERT INTO autonomy_settings (key, value) VALUES ($1, $2)
ON CONFLICT (key) DO UPDATE SET value = $2`,
"destructive_window.agent:"+agentID.String()+".target:"+targetSlug, dExpires)
}
}
slog.Info("httpapi: approved execution queued",

View File

@@ -273,7 +273,7 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server {
InputSchema: objSchema(
prop{"target", "string", "Target entity slug. For pct_create this MUST be the Proxmox HOST that will run the container (e.g. host:strong) — NOT the new LXC's name. For restart/systemctl/apt_upgrade/pct_exec use the target service/LXC slug (e.g. lxc:caddy)."},
prop{"action", "string", "Action: restart, systemctl, pct_exec, apt_upgrade, pct_create"},
prop{"params", "string", "For systemctl: 'enable|disable|reload'. For pct_exec: the shell command. For apt_upgrade: 'audit|upgrade'. For pct_create: a JSON object string with keys: vmid (int, required, unused id), hostname (string, required), cores (int), memory (MB int), disk_gb (int), ip (CIDR e.g. 192.168.8.50/24, or omit/\"dhcp\" — DHCP is the safe default, see below), gw (gateway ip, static only), bridge (e.g. vmbr0/vmbr1 — WHICH BRIDGE REACHES WHICH SUBNET IS DIFFERENT PER HOST, never assume vmbr0; see below), storage (default local-lvm), template (optional — omit to auto-pick newest debian on the host), privileged (bool), nesting (bool), mounts ([]string of 'src,mp=/dst'), services ([]string of apt packages to install), post_install (string shell script run inside the container after create). Example: {\"vmid\":150,\"hostname\":\"typetype\",\"cores\":2,\"memory\":2048,\"disk_gb\":16,\"ip\":\"192.168.8.50/24\",\"gw\":\"192.168.8.2\",\"bridge\":\"vmbr1\",\"nesting\":true,\"services\":[\"docker.io\",\"git\"],\"post_install\":\"git clone https://github.com/x/y /opt/y && cd /opt/y && docker compose up -d\"}. STATIC IP RULE: before setting ip/gw/bridge to anything other than DHCP, use list_entities/get_entity_knowledge to find an EXISTING lxc on the SAME host whose IP is in the same /28 block, and copy its exact gw+bridge — do not invent a gateway. If no such neighbor exists, prefer ip:\"dhcp\" (proven to work, gets a real routable address) over guessing; a wrong bridge/gateway pair fails a fast pre-flight ping check now (seconds, not minutes) but is still a wasted turn — better to not guess at all."},
prop{"params", "string", "For systemctl: 'enable|disable|reload'. For pct_exec: the shell command. For apt_upgrade: 'audit|upgrade'. For pct_create: a JSON object string with keys: vmid (int, required, unused id), hostname (string, required), cores (int), memory (MB int), disk_gb (int), ip (CIDR e.g. 192.168.8.50/24, or omit/\"dhcp\" — DHCP is the safe default, see below), gw (gateway ip, static only), bridge (e.g. vmbr0/vmbr1 — WHICH BRIDGE REACHES WHICH SUBNET IS DIFFERENT PER HOST, never assume vmbr0; see below), storage (default local-lvm), template (optional — omit to auto-pick newest debian on the host), privileged (bool), nesting (bool), mounts ([]string of 'src,mp=/dst'). Example: {\"vmid\":150,\"hostname\":\"typetype\",\"cores\":2,\"memory\":2048,\"disk_gb\":16,\"ip\":\"192.168.8.50/24\",\"gw\":\"192.168.8.2\",\"bridge\":\"vmbr1\",\"nesting\":true}. pct_create is ATOMIC — it ONLY creates and starts the container (no services/post_install params anymore). Once it completes you will be automatically re-invoked with the result; install packages and run setup by issuing your OWN `run` calls against the new lxc:<hostname> target, one step at a time — you'll see each step's real output and can fix exactly the one that fails, instead of one opaque multi-minute install that either fully works or fully doesn't. STATIC IP RULE: before setting ip/gw/bridge to anything other than DHCP, use list_entities/get_entity_knowledge to find an EXISTING lxc on the SAME host whose IP is in the same /28 block, and copy its exact gw+bridge — do not invent a gateway. If no such neighbor exists, prefer ip:\"dhcp\" (proven to work, gets a real routable address) over guessing; a wrong bridge/gateway pair fails a fast pre-flight ping check now (seconds, not minutes) but is still a wasted turn — better to not guess at all."},
),
}, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
args := argsMap(req)
@@ -527,6 +527,28 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server {
return textResult(fmt.Sprintf("run on %s (config_mutation, auto via assent window): %s", targetSlug, out)), nil
}
// Destructive window: a narrow, TARGET-scoped grant opened only after
// an operator's explicit typed confirmation ("I confirm") on this
// same target — never by loose assent. Exists for multi-step
// destructive recovery (e.g. a failed destroy needing stop, then
// destroy) so the operator isn't asked to re-type "I confirm" for
// every single command against the thing they just confirmed.
if riskClass == policy.RiskDestructive && destructiveWindowActive(ctx, pool, agentID, targetSlug) {
host, user, wrap, rerr := resolveExecTarget(ctx, pool, targetSlug)
if rerr != nil {
pool.Exec(ctx, `UPDATE executions SET status='failed', result=$2::jsonb WHERE entity_id=$1`, id, jsonErr("%s", rerr.Error()))
return textResult(fmt.Sprintf("resolve target: %v", rerr)), nil
}
out, xerr := sshExec(ctx, host, user, wrap(command))
if xerr != nil {
pool.Exec(ctx, `UPDATE executions SET status='failed', result=$2::jsonb WHERE entity_id=$1`, id, jsonErr("%s: %s", xerr.Error(), out))
return textResult(fmt.Sprintf("run on %s: ERROR %v\n%s", targetSlug, xerr, out)), nil
}
pool.Exec(ctx, `UPDATE executions SET status='completed', result=$2::jsonb WHERE entity_id=$1`, id, jsonOut(out))
slog.Info("mcp: run auto-executed via destructive window", "target", targetSlug, "execution_id", id)
return textResult(fmt.Sprintf("run on %s (destructive, auto via confirmed-target window): %s", targetSlug, out)), nil
}
pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class=$2 WHERE entity_id=$1`, id, riskClass)
createApproval(ctx, pool, id, targetID, "run", string(runParams), riskClass)
confirmNote := ""
@@ -1384,6 +1406,31 @@ func assentWindowActive(ctx context.Context, pool *db.Pool, agentID uuid.UUID) b
return time.Now().UTC().Before(expires)
}
// destructiveWindowActive reports whether targetSlug has a live, explicitly-
// confirmed destructive grant for this agent. Key format
// ("destructive_window.agent:<id>.target:<slug>") must match
// cmd/nomos/store.go's openDestructiveWindow — both processes read/write the
// same autonomy_settings row. Scoped to one target so a typed confirmation
// for destroying container A can never be read as authorizing anything
// against container B.
func destructiveWindowActive(ctx context.Context, pool *db.Pool, agentID uuid.UUID, targetSlug string) bool {
if agentID == uuid.Nil || targetSlug == "" {
return false
}
var expiresStr string
err := pool.QueryRow(ctx,
"SELECT value FROM autonomy_settings WHERE key = $1",
"destructive_window.agent:"+agentID.String()+".target:"+targetSlug).Scan(&expiresStr)
if err != nil {
return false
}
expires, err := time.Parse(time.RFC3339, expiresStr)
if err != nil {
return false
}
return time.Now().UTC().Before(expires)
}
func createApproval(ctx context.Context, pool *db.Pool, execID, targetID uuid.UUID, action, params, riskClass string) {
p := map[string]any{"action": action, "params": params, "execution_id": execID.String()}
payload, _ := json.Marshal(p)