diff --git a/cmd/codeaf/factory_stage.go b/cmd/codeaf/factory_stage.go index a73c6efc96..3de6a60c69 100644 --- a/cmd/codeaf/factory_stage.go +++ b/cmd/codeaf/factory_stage.go @@ -12,6 +12,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/env" "github.com/Agent-Field/codeaf/internal/factory" factoryrun "github.com/Agent-Field/codeaf/internal/factory/run" @@ -295,6 +296,7 @@ func factoryStagePosture() string { // name) still hold, because they sit under every policy and they are not a // person's preference. A person who wants a stage narrower says so with // [factoryPostureEnv]. +// An unreadable profile still requires a person: its saved denials are unknown. type stageApprovalGate struct { rows *v3ApprovalGate posture string @@ -302,6 +304,13 @@ type stageApprovalGate struct { // Build is the gate for one posture. func (g stageApprovalGate) Build(posture string) (*approval.Policy, bool, error) { + if g.rows != nil { + _, err := config.ReadProfileApprovalConfig(g.rows.profileDir) + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return &approval.Policy{ProfileReadError: err.Error()}, false, nil + } + } if posture != session.PostureAllow { return g.rows.Build(posture) } @@ -486,6 +495,10 @@ func (c *stageConversation) drain(events <-chan session.Event) { } c.say(factory.ToolCaptionPrefix + factory.ToolLogText(hint, c.dir)) case session.EventToolFailed: + var refused *approval.ProfileReadFailure + if errors.As(event.Err, &refused) { + c.fail(refused) + } reason := event.Hint if reason == "" && event.Err != nil { reason = event.Err.Error() @@ -519,7 +532,10 @@ func (c *stageConversation) fail(err error) { } c.mu.Lock() defer c.mu.Unlock() - c.failure = err + var refused *approval.ProfileReadFailure + if !errors.As(c.failure, &refused) { + c.failure = err + } } // say puts one caption on the account, dropping it rather than holding the @@ -631,6 +647,10 @@ func (c *stageConversation) Wait(ctx context.Context) error { case <-done: c.mu.Lock() defer c.mu.Unlock() + var refused *approval.ProfileReadFailure + if errors.As(c.failure, &refused) { + return refused + } if c.budget != nil { if why := c.budget.Refusal(); why != "" { // A daily refusal already recorded was not caused by memory, so diff --git a/cmd/codeaf/factory_stage_profile_job_test.go b/cmd/codeaf/factory_stage_profile_job_test.go new file mode 100644 index 0000000000..d2a43521cc --- /dev/null +++ b/cmd/codeaf/factory_stage_profile_job_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/factory" + factoryrun "github.com/Agent-Field/codeaf/internal/factory/run" + "github.com/Agent-Field/codeaf/internal/factory/store" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/remote" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" +) + +func TestUnreadableStageJobKeepsConfigReason(t *testing.T) { + testUnreadableStageJobKeepsConfigReason(t, false) +} + +func TestInitiallyUnreadableStageJobKeepsConfigReason(t *testing.T) { + testUnreadableStageJobKeepsConfigReason(t, true) +} + +func testUnreadableStageJobKeepsConfigReason(t *testing.T, initiallyUnreadable bool) { + t.Helper() + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + root := resolvedTempDir(t) + t.Setenv("HOME", filepath.Join(root, "home")) + t.Setenv(home.EnvVar, filepath.Join(root, "state")) + t.Setenv(config.ProfileDirEnv, "") + t.Setenv("OPENROUTER_API_KEY", "test-key") + t.Setenv(factoryPostureEnv, "") + workspace := filepath.Join(root, "work") + if err := os.MkdirAll(workspace, 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(workspace, "NOTES.md") + writeArgs, _ := json.Marshal(map[string]string{"path": target, "content": "written by the stage\n"}) + model := newScriptedStageModel(t, stageCall{"write", string(writeArgs)}, stageCall{"stage_result", stageModelReport}) + t.Setenv("CODEAF_BASE_URL", model.server.URL) + t.Chdir(workspace) + freshEngineProcess(t) + stageSettle = 20 * time.Millisecond + t.Cleanup(func() { stageSettle = unattendedRunSettleTick }) + + proc, err := openEngineProcess() + if err != nil { + t.Fatal(err) + } + launch, err := openV3Launch(proc, engineLaunchOptions(remote.Hello{Version: remote.Version, Workspace: workspace}, workspace, "")) + if err != nil { + t.Fatal(err) + } + st, err := store.Open(filepath.Join(root, "factory")) + if err != nil { + t.Fatal(err) + } + id, err := st.Add(context.Background(), factory.Item{ + Title: "write the notes", Repo: "work", Tier: factory.TierOwner, + Stages: []factory.Stage{{Name: "write", Kind: factory.StageChat, Ask: "write the notes", Until: "done", On: true}}, + }) + if err != nil { + t.Fatal(err) + } + if err := st.Update(id, func(it *factory.Item) error { it.Stream = &factory.Stream{}; return nil }); err != nil { + t.Fatal(err) + } + it, _ := st.Get(id) + + profile := "" + file := config.BudgetConfigPath(profile) + if err := os.MkdirAll(filepath.Dir(file), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(file, []byte(`{"tools.approvals":"write:deny"}`), 0600); err != nil { + t.Fatal(err) + } + reason := "saved approval rules could not be read" + if initiallyUnreadable { + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) + } + reason = "permission denied" + } else { + oldHandler := model.server.Config.Handler + var once sync.Once + model.server.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + r.Body = io.NopCloser(bytes.NewReader(body)) + if bytes.Contains(body, []byte(`"stage_result"`)) { + once.Do(func() { + if err := os.Chmod(file, 0); err != nil { + t.Error(err) + } + }) + } + oldHandler.ServeHTTP(w, r) + }) + } + t.Cleanup(func() { _ = os.Chmod(file, 0600) }) + maker := factoryStageMaker(st, workspace, profile, launch.Config) + exec := factoryChatStage(st, profile, factoryrun.NewChatExecutor(maker)) + ctx, cancel := context.WithTimeout(context.Background(), 12*time.Second) + defer cancel() + res, err := exec.Run(ctx, factoryrun.Job{Item: it, Stage: it.Stages[0], Index: 0, Round: 1, Dir: workspace, Log: func(line string) { + t.Logf("JOB LOG: %s", line) + if strings.Contains(line, "reminded") { + t.Error("terminal profile refusal must not retry reporting") + } + }}) + if err != nil { + t.Fatalf("ROUND ERROR: %v", err) + } + t.Logf("ROUND RESULT: %+v", res) + if res.Done || res.Exit != 1 || !strings.Contains(res.Output, file) || !strings.Contains(res.Output, reason) { + t.Errorf("job lost the terminal profile-read reason: %+v", res) + } + model.mu.Lock() + defer model.mu.Unlock() + if initiallyUnreadable { + t.Logf("STAGE MODEL REQUESTS: %d", len(model.briefs)) + if len(model.briefs) != 0 || res.Spent != 0 || res.Reports != 0 { + t.Fatalf("refused admission did work: requests=%d spent=%v reports=%d", len(model.briefs), res.Spent, res.Reports) + } + } + for _, brief := range model.briefs { + if i := strings.Index(brief, "no resolver is attached"); i >= 0 { + t.Logf("MODEL RECEIVED REFUSAL: %.650s", brief[i:]) + } + } + got, readErr := os.ReadFile(target) + t.Logf("TARGET: content=%q err=%v", got, readErr) + if !os.IsNotExist(readErr) { + t.Fatalf("unreadable stage wrote file") + } + if ctx.Err() != nil { + t.Fatalf("stage failed to stop before timeout: %v", ctx.Err()) + } +} diff --git a/cmd/codeaf/factory_stage_unreadable_test.go b/cmd/codeaf/factory_stage_unreadable_test.go new file mode 100644 index 0000000000..2faa70c1d9 --- /dev/null +++ b/cmd/codeaf/factory_stage_unreadable_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/session" +) + +func TestStageUnreadableProfileRequiresPerson(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + profile := v3Profile(t, map[string]any{config.KeyToolApprovalMode: "prompt", config.KeyToolApprovals: "write:deny"}) + file := filepath.Join(profile, "config.json") + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(file, 0o600) }) + rows := &v3ApprovalGate{workspace: t.TempDir(), profileDir: profile, headless: true} + for _, posture := range []string{session.PostureAllow, session.PostureAsk, session.PostureDeny} { + t.Run(posture, func(t *testing.T) { + var gate session.ApprovalGate = stageApprovalGate{rows: rows, posture: posture} + policy, guardian, err := gate.Build(posture) + if err != nil { + t.Fatal(err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"x","content":"x"}`)) + t.Logf("stage write: %s", decision) + if decision.Action != approval.ActionPrompt || guardian || !strings.Contains(decision.Rule, file) || !strings.Contains(decision.Rule, "could not be read") { + t.Fatalf("unreadable saved denial must require a person with a clear reason: decision=%+v guardian=%v", decision, guardian) + } + }) + } +} + +func TestStageProfileReadFailureWinsOverProjectParseError(t *testing.T) { + profile, workspace := t.TempDir(), t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.Mkdir(file, 0o700); err != nil { + t.Fatal(err) + } + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("expected a filesystem read failure") + } + if err := os.Mkdir(filepath.Join(workspace, ".codeaf"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, ".codeaf", "config.json"), []byte("{"), 0o600); err != nil { + t.Fatal(err) + } + for _, posture := range []string{session.PostureAllow, session.PostureAsk, session.PostureDeny, session.PostureGuardian, session.PostureAuto} { + t.Run(posture, func(t *testing.T) { + var gate session.ApprovalGate = stageApprovalGate{rows: &v3ApprovalGate{workspace: workspace, profileDir: profile}, posture: posture} + policy, guardian, err := gate.Build(posture) + if err != nil { + t.Fatalf("known profile failure lost to project error: %v", err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"notes.txt","content":"no"}`)) + if guardian || decision.Action != approval.ActionPrompt || !strings.Contains(decision.ProfileReadError, file) { + t.Fatalf("lost profile failure: guardian=%v decision=%+v", guardian, decision) + } + }) + } +} + +func TestStageReadableMalformedProfileKeepsDefaultAllowance(t *testing.T) { + for _, contents := range []string{"{", "null", "[]"} { + t.Run(contents, func(t *testing.T) { + profile := t.TempDir() + if err := os.WriteFile(filepath.Join(profile, "config.json"), []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + rows := &v3ApprovalGate{workspace: t.TempDir(), profileDir: profile} + var gate session.ApprovalGate = stageApprovalGate{rows: rows, posture: session.PostureAllow} + policy, guardian, err := gate.Build(session.PostureAllow) + if err != nil { + t.Fatal(err) + } + decision := policy.Check("write", json.RawMessage(`{"path":"notes.txt","content":"ok"}`)) + if guardian || decision.Action != approval.ActionAllow || decision.ProfileReadError != "" { + t.Fatalf("readable default-stage bypass changed: guardian=%v decision=%+v", guardian, decision) + } + if _, _, err := gate.Build(session.PostureAsk); err == nil { + t.Fatal("readable malformed JSON must still fail ordinary row construction") + } + }) + } +} diff --git a/docs/changes/unreleased/2325-unreadable-config-limits.md b/docs/changes/unreleased/2325-unreadable-config-limits.md new file mode 100644 index 0000000000..a6095dd0f2 --- /dev/null +++ b/docs/changes/unreleased/2325-unreadable-config-limits.md @@ -0,0 +1,25 @@ +--- +kind: fixed +title: codeaf keeps unreadable profile approval limits in factory stages and task children +pr: 2325 +surface: [engine, factory] +invalidates: + - "A factory stage's allow posture bypassed an unreadable profile's saved approval rules. It now requires a person when those rules cannot be read." + - "Task workers, repair rounds and auditors dropped their parent's profile read failure. They now consult that signal at each decision and refuse calls that need a person when nobody can answer." + - "A project parse error could hide a known profile read failure. The stage now returns the failure already in hand before reading project rules." + - "Standalone run workers discarded profile read failures. They now check their run's profile before each call, including after work starts." + - "A refused job could lose the config-file reason during report recovery or first-turn budget admission. Its result now keeps that reason without futile report retries." +--- + +A readable profile keeps its existing behavior, including a factory stage's own +allow posture. An unreadable profile cannot silently discard a saved denial: +the existing approval gate reports the failed file read and requires a person. + +The named readable default-stage bypass includes readable malformed JSON. +Other postures retain their existing parse-error behavior. Filesystem read +failures remain distinct from JSON parse failures. Task, repair, checker and +fleet children keep their own normal allowance while following the parent's +current failed-read signal; recovery clears that inherited signal too. + +First-turn daily-limit refusals retain the config filename and read error. +Admission decisions, spending counts and charges are unchanged. diff --git a/internal/approval/approval.go b/internal/approval/approval.go index 15559e39dc..064d8c59b2 100644 --- a/internal/approval/approval.go +++ b/internal/approval/approval.go @@ -459,3 +459,9 @@ func toList(value any) ([]any, bool) { return nil, false } } + +// ProfileReadFailure is an unattended call refused because the saved rules +// could not be read. Runners retain this reason ahead of report recovery. +type ProfileReadFailure struct{ Reason string } + +func (e *ProfileReadFailure) Error() string { return e.Reason } diff --git a/internal/factory/run/chatstage.go b/internal/factory/run/chatstage.go index da8e58cb67..daad5e983d 100644 --- a/internal/factory/run/chatstage.go +++ b/internal/factory/run/chatstage.go @@ -45,6 +45,7 @@ import ( "sync" "time" + "github.com/Agent-Field/codeaf/internal/approval" "github.com/Agent-Field/codeaf/internal/factory" "github.com/Agent-Field/codeaf/internal/factory/verification" ) @@ -402,6 +403,11 @@ loop: // one that is idle with nothing reported is asked again (stagereport.go) // until there is nothing left to ask. A spending limit is never asked // past: the turn it would start is refused before it begins. + var refused *approval.ProfileReadFailure + if errors.As(waitErr, &refused) { + idleFailure = &factory.StageResult{Exit: 1, Output: refused.Error()} + break loop + } if _, stopped := spendStopOf(waitErr); stopped { break loop } diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index 414b25e257..47db2cf0db 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -611,12 +611,20 @@ whatever posture or `--yolo` you selected. The prompt names the file and explain that saved approval rules could not be read. Remembered approvals and the guardian cannot answer these questions for you. A headless call with nobody to ask returns an error naming the file. Once the file is readable, its rules apply again. -Malformed JSON still fails policy construction. - -Two kinds of work do not get this protection yet. A default Factory stage runs on -its own allow posture and does not read your saved rules, whether the file is -readable or not. A task child started by a conversation does not carry the failed -read over from its parent, so it can run a call your saved rules would deny. +Malformed JSON still fails ordinary policy construction; the readable default-stage +bypass below also applies to malformed JSON. + +Factory stages and work started by tasks get the same protection: every call +needs a person, and with nobody to ask it is refused. Workers, repair rounds and +checkers see a parent's current read failure even after they start. Standalone +run workers check the profile before each call. A refused Factory job keeps the +failed-read reason and config filename in its result, without report retries. +This includes a daily-limit check that refuses the first turn because the +profile cannot be read; no model request is made. + +**Readable default-stage bypass:** with a readable file, a default Factory stage +runs on its own allow posture and does not apply your saved rules. This named +limit also covers readable malformed JSON. ## Default YOLO in interactive conversations and headless runs diff --git a/internal/run/bashworker.go b/internal/run/bashworker.go index dcae2cf52e..771ec22bf9 100644 --- a/internal/run/bashworker.go +++ b/internal/run/bashworker.go @@ -58,9 +58,10 @@ import ( // CODEAF_TASK_BELT names the older belt, so on that road not one byte of any // prompt, belt or landing changes — nothing constructs this worker. type BashWorker struct { - store *plandb.Store - workspace string - model string + profileDir string + store *plandb.Store + workspace string + model string // always is the memory section over this place — every line kept for the // person, this machine and this project — rendered by the door // ([session.MemoryWorld]) and closed on by the worker's brief. It is the @@ -177,6 +178,7 @@ func (w *BashWorker) Run(ctx context.Context, task plandb.Task) (rep Report, run } agent, err := session.NewBeltWorker(session.Config{ Effort: w.taskEffort, + ApprovalGate: session.BeltWorkerApprovalGate(w.profileDir), Workspace: w.workspace, WorkspaceURI: w.workspaceURI, WorkspaceSet: w.workspaceSet.Clone(), diff --git a/internal/run/bashworker_profile_test.go b/internal/run/bashworker_profile_test.go new file mode 100644 index 0000000000..16b32a1ae6 --- /dev/null +++ b/internal/run/bashworker_profile_test.go @@ -0,0 +1,66 @@ +package run_test + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/run" + "github.com/Agent-Field/codeaf/internal/session" +) + +func TestCrewWorkerReadsCurrentProfileBeforeEachCall(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_PLANDB_BIN", stubCLI(t)) + t.Setenv("CODEAF_HOME", t.TempDir()) + profile, workspace := t.TempDir(), t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.WriteFile(file, []byte(`{}`), 0o600); err != nil { + t.Fatal(err) + } + var refused bool + s := &seat{script: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolReply(`{"command":"printf BEFORE > before.txt"}`), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + if err := os.Remove(file); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(file, 0o700); err != nil { + t.Fatal(err) + } + return toolReply(`{"command":"printf AFTER > after.txt"}`), nil + }, + func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + for _, m := range messages { + text := messageContent(m) + if m.Role == "tool" && strings.Contains(text, file) && strings.Contains(text, "saved approval rules could not be read") { + refused = true + } + } + return nil, errors.New("API error (401): fixture complete") + }, + }} + store := runOpenStore(t) + factory := run.CrewFactory(store, workspace, profile, run.Seats{One: "test/model"}, "", false, func(string) session.Completer { return s }) + worker := factory(*store.Task(store.RootID())) + _, err := worker.Run(runContext(t), *store.Task(store.RootID())) + if err == nil { + t.Fatal("scripted provider should end with its authentication refusal") + } + t.Logf("scripted provider ending: %v", err) + if _, err := os.Stat(filepath.Join(workspace, "before.txt")); err != nil { + t.Fatalf("readable worker failed: %v", err) + } + if _, err := os.Stat(filepath.Join(workspace, "after.txt")); !os.IsNotExist(err) { + t.Errorf("worker wrote after failed read: %v", err) + } + if !refused { + t.Fatal("worker lost config filename and failed-read reason") + } +} diff --git a/internal/run/crew.go b/internal/run/crew.go index 05cc02339c..0b72ea8b72 100644 --- a/internal/run/crew.go +++ b/internal/run/crew.go @@ -151,6 +151,7 @@ func crewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, dailyBudgetUSD, _ := config.DailyBudgetUSDAt(profileDir) workerFor := func(model string, completer session.Completer) *BashWorker { worker := NewBashWorker(store, workspace, model, completer, workerOpts...) + worker.profileDir = profileDir worker.dailyBudgetUSD = dailyBudgetUSD worker.spendPreauthorized = preauthorized // The memory section rides on every worker this run seats, read once by diff --git a/internal/session/agent.go b/internal/session/agent.go index d35cc7ac46..b8a537b46e 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -118,6 +118,7 @@ func (a *Agent) newChildAgent(config Config) (*Agent, error) { if err != nil { return nil, err } + child.approvalReadParent = a child.managedClient = managed child.clientAccount = account child.clientPool = pool diff --git a/internal/session/approvalgate.go b/internal/session/approvalgate.go index 0220fa6ff1..34c0b230dc 100644 --- a/internal/session/approvalgate.go +++ b/internal/session/approvalgate.go @@ -62,6 +62,12 @@ func (a *Agent) SetApprovalPolicy(policy *approval.Policy) { // [Agent.SetApprovalPolicy] existed, a single unguarded read became a race the // detector finds in a second and a person would never reproduce. func (a *Agent) approvalGate() *approval.Policy { + if a.approvalReadParent != nil { + if parent := a.approvalReadParent.approvalGate(); parent != nil && parent.ProfileReadError != "" { + return &approval.Policy{ProfileReadError: parent.ProfileReadError} + } + } + if source, ok := a.jobBudgetOf().(interface{ FactoryAuto() bool }); ok && source.FactoryAuto() { return automaticJobPolicy(a.baseApprovalGate()) } diff --git a/internal/session/bashbelt_profile_test.go b/internal/session/bashbelt_profile_test.go new file mode 100644 index 0000000000..4002af4faf --- /dev/null +++ b/internal/session/bashbelt_profile_test.go @@ -0,0 +1,52 @@ +package session + +import ( + "context" + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/plandb" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBeltWorkerKeepsProfileReadFailure(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv(planCLIBinEnv, filepath.Join(t.TempDir(), "stub-codeaf")) + profile := t.TempDir() + if err := os.Mkdir(filepath.Join(profile, "config.json"), 0o700); err != nil { + t.Fatal(err) + } + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("expected read failure") + } + dir := t.TempDir() + store, err := plandb.Open(filepath.Join(dir, planStoreFilename), "the work", planRootID, "", "") + if err != nil { + t.Fatal(err) + } + workspace := t.TempDir() + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", "bash", `{"command":"printf WRITTEN > denied.txt"}`), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + cfg := Config{Workspace: workspace, Model: "test/model", System: "SYSTEM", ApprovalPolicy: &approval.Policy{ProfileReadError: readErr.Error()}} + child, err := NewBeltWorker(cfg, script, store.Task(planRootID), store.Path(), store.RootID()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + events := collect(t, mustSubmit(t, child, "write")) + failed, ok := firstOfKind(events, EventToolFailed) + _, statErr := os.Stat(filepath.Join(workspace, "denied.txt")) + t.Logf("belt refusal=%v reason=%q file_exists=%v", ok, failed.Output, statErr == nil) + if !ok || !strings.Contains(failed.Output, readErr.Error()) || !os.IsNotExist(statErr) { + t.Fatal("belt worker lost unreadable signal") + } +} diff --git a/internal/session/bashbelt_worker.go b/internal/session/bashbelt_worker.go index e10c177e40..e443c20bf2 100644 --- a/internal/session/bashbelt_worker.go +++ b/internal/session/bashbelt_worker.go @@ -32,6 +32,7 @@ import ( "strings" "github.com/Agent-Field/codeaf/internal/approval" + profileconfig "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/effort" "github.com/Agent-Field/codeaf/internal/plandb" "github.com/Agent-Field/codeaf/internal/router" @@ -90,7 +91,14 @@ func NewBeltWorker(config Config, completer Completer, task *plandb.Task, storeP // allow into a prompt for the shapes that destroy a disk or drop the // machine, and a prompt in a worker is a refusal it can read — never a // question and never a hang. - config.ApprovalPolicy = &approval.Policy{Default: approval.ActionAllow} + policy := &approval.Policy{Default: approval.ActionAllow} + if config.ApprovalPolicy != nil { + policy.ProfileReadError = config.ApprovalPolicy.ProfileReadError + } + config.ApprovalPolicy = policy + if config.ApprovalGate == nil { + config.ApprovalGate = beltWorkerApprovalGate{policy: policy, profileDir: config.ProfileDir} + } config.AskConsent = false // THE WORKER'S OWN RECORD FOLDER. The transcript this agent writes and the // whole outputs the belt's cut files land in the task's own folder beside @@ -365,3 +373,30 @@ func (a *Agent) sendBeltStep(ctx context.Context, hub *eventHub, event Event) { case <-ctx.Done(): } } + +// BeltWorkerApprovalGate supplies live failed-read protection to a standalone +// run worker without giving unrelated session settings a different profile. +func BeltWorkerApprovalGate(profileDir string) ApprovalGate { + return beltWorkerApprovalGate{profileDir: profileDir, policy: &approval.Policy{Default: approval.ActionAllow}} +} + +// beltWorkerApprovalGate checks the run's profile at each decision because a +// standalone run worker has no parent agent whose current gate it can consult. +// Readable files retain the worker's own allowance, including parse failures. +type beltWorkerApprovalGate struct { + policy *approval.Policy + profileDir string +} + +func (g beltWorkerApprovalGate) Standing() string { + return PostureAllow +} + +func (g beltWorkerApprovalGate) Build(_ string) (*approval.Policy, bool, error) { + _, err := profileconfig.ReadProfileApprovalConfig(g.profileDir) + var pathErr *os.PathError + if errors.As(err, &pathErr) { + return &approval.Policy{ProfileReadError: err.Error()}, false, nil + } + return g.policy, false, nil +} diff --git a/internal/session/child_profile_live_test.go b/internal/session/child_profile_live_test.go new file mode 100644 index 0000000000..babe7a99f2 --- /dev/null +++ b/internal/session/child_profile_live_test.go @@ -0,0 +1,152 @@ +package session + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/orchestrate" +) + +func newProfileTestChild(t *testing.T, parent *Agent, kind, workspace string) *Agent { + t.Helper() + g := parent.graph() + g.run = func(*TaskNode) {} + id := g.reserve() + g.admit(id, taskSpec{title: "work", brief: "answer", acceptance: "done"}) + var child *Agent + var err error + switch kind { + case "worker": + child, err = parent.newTaskAgent(context.Background(), workspace, g.node(id), "") + case "repair": + child, err = parent.newTaskAgentOn(context.Background(), workspace, g.node(id), "-repair1", "", true) + case "auditor": + child, err = parent.newAuditAgent(workspace, g.node(id), plainDoor(auditReadCommands), "") + case "fleet": + executor := &orchestrateExec{agent: parent, id: "work"} + child, err = executor.newChild(workspace, orchestrate.Node{ID: "n1", Goal: "answer"}) + default: + t.Fatalf("unknown child kind %q", kind) + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + return child +} + +func profileTestReadFailure(t *testing.T) string { + t.Helper() + profile := t.TempDir() + if err := os.Mkdir(filepath.Join(profile, "config.json"), 0o700); err != nil { + t.Fatal(err) + } + _, err := config.ReadProfileApprovalConfig(profile) + if err == nil { + t.Fatal("expected filesystem read failure") + } + return err.Error() +} + +func TestChildrenObserveCurrentProfileReadFailure(t *testing.T) { + for _, kind := range []string{"worker", "repair", "auditor", "fleet"} { + for _, after := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/failed_after_creation=%v", kind, after), func(t *testing.T) { + tool, args := "write", `{"path":"denied.txt","content":"must not be written"}` + if kind == "auditor" { + tool, args = "bash", `{"command":"pwd"}` + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", tool, args), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + parent, _ := newTestAgent(t, script, func(c *Config) { c.OneModel = true; c.ApprovalPolicy = &approval.Policy{Default: approval.ActionAllow} }) + readError := profileTestReadFailure(t) + failed := &approval.Policy{ProfileReadError: readError} + if !after { + parent.SetApprovalPolicy(failed) + } + workspace := t.TempDir() + child := newProfileTestChild(t, parent, kind, workspace) + if after { + parent.SetApprovalPolicy(failed) + } + events := collect(t, mustSubmit(t, child, "attempt the call")) + refusal, ok := firstOfKind(events, EventToolFailed) + if !ok || !strings.Contains(refusal.Output, readError) { + t.Fatalf("lost current profile failure: events=%v refusal=%q", kinds(events), refusal.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "denied.txt")); !os.IsNotExist(err) { + t.Fatalf("refused write created file: %v", err) + } + }) + } + } +} + +func TestChildObservesProfileFailureDuringTurn(t *testing.T) { + for _, kind := range []string{"worker", "repair", "auditor", "fleet"} { + for _, stage := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/stage=%v", kind, stage), func(t *testing.T) { + readError := profileTestReadFailure(t) + // The child must keep its own allowance even when the parent's rules deny calls. + normal := &approval.Policy{Default: approval.ActionDeny} + var parent *Agent + call := func(id string) *ai.Response { + if kind == "auditor" { + return toolResponse(id, "bash", `{"command":"pwd"}`) + } + return toolResponse(id, "write", fmt.Sprintf(`{"path":%q,"content":"written"}`, id+".txt")) + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { return call("before"), nil }, + func(context.Context, []ai.Message) (*ai.Response, error) { + parent.SetApprovalPolicy(&approval.Policy{ProfileReadError: readError}) + return call("after"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + parent.SetApprovalPolicy(normal) + return call("recovered"), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + parent, _ = newTestAgent(t, script, func(c *Config) { + c.OneModel = true + c.ApprovalPolicy = normal + if stage { + c.Stage = &fakeStageDoor{} + c.Unattended = true + } + }) + workspace := t.TempDir() + child := newProfileTestChild(t, parent, kind, workspace) + events := collect(t, mustSubmit(t, child, "attempt three calls")) + succeeded := make(map[string]bool) + var refusal Event + for _, event := range events { + if event.Kind == EventToolEnd { + succeeded[event.CallID] = true + } + if event.Kind == EventToolFailed && event.CallID == "after" { + refusal = event + } + } + if !succeeded["before"] || !succeeded["recovered"] || succeeded["after"] || !strings.Contains(refusal.Output, readError) { + t.Fatalf("child must allow, refuse, then allow on its own rules: succeeded=%v refusal=%q", succeeded, refusal.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "after.txt")); !os.IsNotExist(err) { + t.Fatalf("refused write created file: %v", err) + } + }) + } + } +} diff --git a/internal/session/consent.go b/internal/session/consent.go index 7dd1c572b7..d43e3a936c 100644 --- a/internal/session/consent.go +++ b/internal/session/consent.go @@ -294,14 +294,14 @@ func (a *Agent) approve(ctx context.Context, hub *eventHub, call ai.ToolCall) (t // worker with no colleague in the room is that this one needed a person and // there is not one. if a.config.InTask { - return refusal("refused in a task: " + decision.Rule + " — nobody to ask"), false + return approvalRefusal("refused in a task: "+decision.Rule+" — nobody to ask", decision), false } // Nobody is watching. Denying is the only honest answer: blocking would // hang a headless run forever on a question with no reader, and allowing // would make "prompt" mean "allow" wherever the surface is not a terminal. if !a.config.AskConsent || hub == nil { - return refusal("needs approval but no resolver is attached: " + decision.Rule), false + return approvalRefusal("needs approval but no resolver is attached: "+decision.Rule, decision), false } allowed, err := a.ask(ctx, hub, call, decision) @@ -557,6 +557,14 @@ func (a *Agent) PendingConsent() []uint64 { return ids } +func approvalRefusal(reason string, decision approval.Decision) toolResult { + result := refusal(reason) + if decision.ProfileReadError != "" { + result.err = &approval.ProfileReadFailure{Reason: reason} + } + return result +} + // refusal is one refused call as the model sees it: an error result, with the // reason in the words the person would read. func refusal(reason string) toolResult { diff --git a/internal/session/loop.go b/internal/session/loop.go index 79f26d812e..96bc48d812 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -3231,6 +3231,7 @@ func backoffWait(ctx context.Context, delay time.Duration) error { // ── tools ─────────────────────────────────────────────────────────────────── type toolResult struct { + err error text string isError bool interruption string @@ -3695,6 +3696,7 @@ func (a *Agent) runToolsWarm(ctx context.Context, ep *episode, calls []ai.ToolCa if results[index].isError { a.sendBeltStep(ctx, hub, Event{ Kind: EventToolFailed, + Err: results[index].err, Tool: call.Function.Name, Hint: clip(firstLine(results[index].text), hintLimit), Args: rendered[index], diff --git a/internal/session/rail.go b/internal/session/rail.go index 4f22797a8d..604a1c5c8e 100644 --- a/internal/session/rail.go +++ b/internal/session/rail.go @@ -116,7 +116,7 @@ func (a *Agent) railBlockLocked() error { // on a guess. daily, err := a.dailyCeiling() if err != nil { - return spendUnverified{said: "daily limit could not be read · /budget day sets it"} + return spendUnverified{said: fmt.Sprintf("daily limit could not be read: %v · /budget day sets it", err)} } if daily > 0 { spentToday, err := spentTodayOnLedgerChecked() diff --git a/internal/session/session.go b/internal/session/session.go index 5a8ad2811b..48a21ea5b1 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2802,8 +2802,10 @@ type Agent struct { crewDayHeld *SpendDay // Clarification streams and their deferred history share the agent lock. // questionParent is installed before a child becomes reachable. - questionParent func(Event) - approvalParent *Agent + questionParent func(Event) + approvalParent *Agent + // approvalReadParent shares only failed reads; the child keeps its own rules. + approvalReadParent *Agent discussionEvents []Event discussionEventSeq uint64 discussionRecorded map[string]bool diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index e91d95bead..ffd71de6cb 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -3211,6 +3211,7 @@ func (a *Agent) newAuditAgent(dir string, node *TaskNode, door auditDoor, on str // belt is what actually constrains this agent: there is no hand here // that writes. AskConsent is off and InTask is on for the node's own // reason — there is nobody in a worktree to ask. + // An unreadable profile still requires a person, just as it does for a worker. ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, AskConsent: false, InTask: true, diff --git a/internal/session/task_profile_approval_test.go b/internal/session/task_profile_approval_test.go new file mode 100644 index 0000000000..459a9bc556 --- /dev/null +++ b/internal/session/task_profile_approval_test.go @@ -0,0 +1,90 @@ +package session + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/config" +) + +func TestTaskChildrenKeepUnreadableProfileApproval(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("mode 000 requires nonroot") + } + profile := t.TempDir() + file := filepath.Join(profile, "config.json") + if err := os.WriteFile(file, []byte(`{"tools.approvalMode":"prompt","tools.approvals":"write:deny"}`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(file, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(file, 0o600) }) + _, readErr := config.ReadProfileApprovalConfig(profile) + if readErr == nil { + t.Fatal("mode 000 profile unexpectedly readable") + } + for _, kind := range []string{"worker", "repair", "auditor"} { + for _, pushed := range []bool{false, true} { + name := kind + "/configured" + if pushed { + name = kind + "/pushed" + } + t.Run(name, func(t *testing.T) { + tool, args := "write", `{"path":"denied.txt","content":"must not be written"}` + if kind == "auditor" { + tool, args = "bash", `{"command":"pwd"}` + } + script := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("attempt", tool, args), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + policy := &approval.Policy{ProfileReadError: readErr.Error()} + parent, _ := newTestAgent(t, script, func(c *Config) { + c.OneModel = true + if !pushed { + c.ApprovalPolicy = policy + } + }) + if pushed { + parent.SetApprovalPolicy(policy) + } + g := parent.graph() + g.run = func(*TaskNode) {} + id := g.reserve() + g.admit(id, taskSpec{title: "work", brief: "answer", acceptance: "done"}) + workspace := t.TempDir() + var child *Agent + var err error + switch kind { + case "auditor": + child, err = parent.newAuditAgent(workspace, g.node(id), plainDoor(auditReadCommands), "") + case "repair": + child, err = parent.newTaskAgentOn(context.Background(), workspace, g.node(id), "-repair1", "", true) + default: + child, err = parent.newTaskAgent(context.Background(), workspace, g.node(id), "") + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + events := collect(t, mustSubmit(t, child, "attempt the call")) + failed, ok := firstOfKind(events, EventToolFailed) + t.Logf("child %s: failed=%v output=%q", tool, ok, failed.Output) + if !ok || !strings.Contains(failed.Output, "saved approval rules could not be read") || !strings.Contains(failed.Output, readErr.Error()) { + t.Errorf("child lost failed-read reason: events=%v refusal=%q", kinds(events), failed.Output) + } + if _, err := os.Stat(filepath.Join(workspace, "denied.txt")); !os.IsNotExist(err) { + t.Errorf("refused write created file: %v", err) + } + }) + } + } +} diff --git a/internal/session/task_run.go b/internal/session/task_run.go index 5e6d809134..71d60a06a1 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -65,10 +65,11 @@ package session // ── WHY A NODE NEVER ASKS ── // // There is nobody to ask. Its approval posture is composed from the same -// [approval.Policy] the conversation uses — allow everything, with the critical +// approval machinery the conversation uses — allow everything, with the critical // table (rm -rf /, mkfs, a redirect onto a raw disk, shutdown) still a floor -// under it — and where a conversation would raise a question, the node reads a -// refusal it can act on. That is exactly consent.go's law for a headless run, +// under it and any profile read failure carried forward. Where a conversation +// would raise a question, the node reads a refusal it can act on. +// That is exactly consent.go's law for a headless run, // said in the node's own words. import ( @@ -7898,6 +7899,7 @@ func (a *Agent) newTaskAgentOn(ctx context.Context, dir string, node *TaskNode, // turns an allow into a "prompt" for the handful of shapes that destroy // a disk or drop the machine, and a prompt in a node is a refusal it can // read (consent.go, InTask) — never a question and never a hang. + // An unreadable profile also requires a person because saved denials are unknown. ApprovalPolicy: &approval.Policy{Default: approval.ActionAllow}, AskConsent: false, InTask: true,