From a19760a6072d829d2c537806ab4786987fb0faa7 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Sat, 29 Aug 2026 14:22:12 -0400 Subject: [PATCH] feat: the shell step can run inside a Configure-authored environment (goal 0240 S4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit process-shell-command gains an optional Execution environment reference (RefKind execenv) — empty keeps S1's documented posture (your real login shell and real environment, byte-for-byte); set, the block runs under that ExecEnv through the SAME machinery code-execution uses (shellArgv's clean/login flags, resolveDir's per-block temp-dir mint, explicit-only env with the minimal-PATH default), and block-referenced secrets upsert onto the environment's explicit env, never os.Environ. The Confirm screen names the environment (a secret-free preview seam — never the vault-resolving lookup, so previews can't trigger audit lines). New typedfield contract: OptionalRef marks a RefKind whose empty value is a deliberate posture, skipped by preflight's required-ref check. No new entity: ExecEnv already IS the profile the goal's session answer described. Coding-loop seed revision bumped (its node config grew the resolved default); contract schemas regenerated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012im1JxQQV2ahnXzZDdVmZq --- .../internal/adapters/openapispec/models.ts | 11 +++ .../mill/internal/domain/typedfield/models.ts | 11 +++ .../internal/services/codeloopsvc/models.ts | 8 ++ .../services/pluginsvc/pluginservice.ts | 3 +- frontend/src/locales/en/app.json | 3 +- .../src/shared/CodingLoopConfirmState.tsx | 4 +- internal/contract/contract.json | 52 +++++++++- internal/contract/schemas/atlas.schema.json | 3 + .../contract/schemas/decision.schema.json | 3 + internal/contract/schemas/list.schema.json | 3 + .../contract/schemas/workflow.schema.json | 3 + .../builtinworkflows_codingloop.go | 2 +- internal/domain/composition/codeexec.go | 4 + .../composition/codingloopsecrets_test.go | 1 + .../domain/composition/executeshellcommand.go | 82 ++++++++++++++-- .../executeshellcommand_execenv_test.go | 96 +++++++++++++++++++ internal/domain/composition/graphpreflight.go | 2 +- internal/domain/typedfield/typedfield.go | 8 ++ .../services/codeloopsvc/codeloopservice.go | 27 ++++++ .../codeloopservice_shellguard_test.go | 24 +++++ .../services/configuresvc/configureexecenv.go | 2 +- .../services/seeding/seed_fingerprints.json | 4 +- internal/services/wiring/wiring.go | 38 ++++++++ main.go | 1 + userdocs/llms-full.txt | 4 +- userdocs/reference/steps.md | 4 +- 26 files changed, 382 insertions(+), 21 deletions(-) create mode 100644 internal/domain/composition/executeshellcommand_execenv_test.go diff --git a/frontend/bindings/github.com/alicoding/mill/internal/adapters/openapispec/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/adapters/openapispec/models.ts index 7283c0bcb..f76e2aa51 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/adapters/openapispec/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/adapters/openapispec/models.ts @@ -80,6 +80,17 @@ export interface Field { */ "RefKind": string; + /** + * OptionalRef marks a RefKind field whose EMPTY value is a legal, + * meaningful configuration rather than "not configured yet" + * (docs/goals/0240 S4's shell-step environment: empty means the + * user's real login shell, a deliberate posture, not a gap) -- + * graph preflight skips its required-ref check. Meaningless + * without RefKind. JSON-tagged omitempty for the same + * byte-identical-marshaling reason Deprecated below documents. + */ + "OptionalRef"?: boolean; + /** * ShowOnCard surfaces this field's value on a card's compact face * (docs/goals/0152) -- read by Atlas Kinds only; other diff --git a/frontend/bindings/github.com/alicoding/mill/internal/domain/typedfield/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/domain/typedfield/models.ts index 33fc7db78..b5767324a 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/domain/typedfield/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/domain/typedfield/models.ts @@ -66,6 +66,17 @@ export interface Field { */ "RefKind": string; + /** + * OptionalRef marks a RefKind field whose EMPTY value is a legal, + * meaningful configuration rather than "not configured yet" + * (docs/goals/0240 S4's shell-step environment: empty means the + * user's real login shell, a deliberate posture, not a gap) -- + * graph preflight skips its required-ref check. Meaningless + * without RefKind. JSON-tagged omitempty for the same + * byte-identical-marshaling reason Deprecated below documents. + */ + "OptionalRef"?: boolean; + /** * ShowOnCard surfaces this field's value on a card's compact face * (docs/goals/0152) -- read by Atlas Kinds only; other diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/codeloopsvc/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/codeloopsvc/models.ts index ab5722c5f..1bdf35bf5 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/codeloopsvc/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/codeloopsvc/models.ts @@ -9,6 +9,14 @@ export interface CommandBlockPreview { "shell": string; "dir": string; + /** + * EnvironmentLabel names the Configure-authored execution + * environment the seeded shell step is configured to run inside + * (docs/goals/0240 S4) -- empty in the default real-login-shell + * posture, where Shell/Dir above already tell the whole story. + */ + "environmentLabel": string; + /** * GuardrailVerdict is "allow" | "ask" | "deny" -- the block-level * gate decision (goal 0240 S3): the MOST RESTRICTIVE of every step's diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/pluginsvc/pluginservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/pluginsvc/pluginservice.ts index b899e4294..ec5b1be33 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/pluginsvc/pluginservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/pluginsvc/pluginservice.ts @@ -3,7 +3,8 @@ /** * PluginService is Wails-bound. openURL is injected so tests never - * shell out to the real OS handler. + * shell out to the real OS handler. appVersion is the build-stamped + * Mill version minMillVersion enforcement compares against. * @module */ diff --git a/frontend/src/locales/en/app.json b/frontend/src/locales/en/app.json index 8ef11dd53..62b58b959 100644 --- a/frontend/src/locales/en/app.json +++ b/frontend/src/locales/en/app.json @@ -275,7 +275,8 @@ }, "startFailedTitle": "Couldn't start the run", "cancel": "Cancel", - "run": "Run" + "run": "Run", + "targetEnv": "Runs in the {{env}} environment ({{shell}}, {{dir}})" }, "running": { "status": { diff --git a/frontend/src/shared/CodingLoopConfirmState.tsx b/frontend/src/shared/CodingLoopConfirmState.tsx index 95a9aeffe..c3d4b5cab 100644 --- a/frontend/src/shared/CodingLoopConfirmState.tsx +++ b/frontend/src/shared/CodingLoopConfirmState.tsx @@ -55,7 +55,9 @@ export function CodingLoopConfirmState({ preview, previewError, startError, type {t('codingLoop.confirm.summary', { count: stepCount, plural: stepCount === 1 ? '' : 's' })} - {t('codingLoop.confirm.target', { shell: preview.shell, dir: preview.dir })} + {preview.environmentLabel + ? t('codingLoop.confirm.targetEnv', { env: preview.environmentLabel, shell: preview.shell, dir: preview.dir }) + : t('codingLoop.confirm.target', { shell: preview.shell, dir: preview.dir })}
    diff --git a/internal/contract/contract.json b/internal/contract/contract.json index 8b9f40534..c17ff3c73 100644 --- a/internal/contract/contract.json +++ b/internal/contract/contract.json @@ -81,6 +81,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, @@ -402,6 +405,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, @@ -587,6 +593,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, @@ -1156,6 +1165,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, @@ -2626,8 +2638,24 @@ "ID": "process-shell-command", "Kind": "process", "Label": "Run a captured command", - "Description": "Runs the captured payload in your real login shell, exactly as written -- no sandboxing, no stored environment profile. A piped command stays one step; commands separated by a new line or \u0026\u0026 show as separate steps. External effect -- the run asks for your approval by default. No configurable fields: this node always runs whatever payload it receives.", - "ConfigFields": null, + "Description": "Runs the captured payload exactly as written -- in your real login shell by default, or inside a Configure-authored execution environment (its shell, directory, and variables) when one is chosen. A piped command stays one step; commands separated by a new line or \u0026\u0026 show as separate steps. External effect -- the run asks for your approval by default.", + "ConfigFields": [ + { + "Key": "envId", + "Label": "Execution environment", + "Type": "text", + "Required": false, + "Default": "", + "Description": "Runs the block inside a Configure-authored environment. Empty runs your real login shell.", + "Options": null, + "Suggestions": null, + "Secret": false, + "RefKind": "execenv", + "OptionalRef": true, + "Multiline": false, + "SystemManaged": false + } + ], "Output": "combined stdout+stderr from every sub-command that ran", "Consumes": [ "text" @@ -4823,8 +4851,24 @@ "ID": "process-shell-command", "Kind": "process", "Label": "Run a captured command", - "Description": "Runs the captured payload in your real login shell, exactly as written -- no sandboxing, no stored environment profile. A piped command stays one step; commands separated by a new line or \u0026\u0026 show as separate steps. External effect -- the run asks for your approval by default. No configurable fields: this node always runs whatever payload it receives.", - "ConfigFields": null, + "Description": "Runs the captured payload exactly as written -- in your real login shell by default, or inside a Configure-authored execution environment (its shell, directory, and variables) when one is chosen. A piped command stays one step; commands separated by a new line or \u0026\u0026 show as separate steps. External effect -- the run asks for your approval by default.", + "ConfigFields": [ + { + "Key": "envId", + "Label": "Execution environment", + "Type": "text", + "Required": false, + "Default": "", + "Description": "Runs the block inside a Configure-authored environment. Empty runs your real login shell.", + "Options": null, + "Suggestions": null, + "Secret": false, + "RefKind": "execenv", + "OptionalRef": true, + "Multiline": false, + "SystemManaged": false + } + ], "Output": "combined stdout+stderr from every sub-command that ran", "Consumes": [ "text" diff --git a/internal/contract/schemas/atlas.schema.json b/internal/contract/schemas/atlas.schema.json index a245d08cd..832efb591 100644 --- a/internal/contract/schemas/atlas.schema.json +++ b/internal/contract/schemas/atlas.schema.json @@ -46,6 +46,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, diff --git a/internal/contract/schemas/decision.schema.json b/internal/contract/schemas/decision.schema.json index bef8a1204..c066f3d44 100644 --- a/internal/contract/schemas/decision.schema.json +++ b/internal/contract/schemas/decision.schema.json @@ -46,6 +46,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, diff --git a/internal/contract/schemas/list.schema.json b/internal/contract/schemas/list.schema.json index 57b6ac51e..ac33f5f8d 100644 --- a/internal/contract/schemas/list.schema.json +++ b/internal/contract/schemas/list.schema.json @@ -46,6 +46,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, diff --git a/internal/contract/schemas/workflow.schema.json b/internal/contract/schemas/workflow.schema.json index c8a09c895..835b05603 100644 --- a/internal/contract/schemas/workflow.schema.json +++ b/internal/contract/schemas/workflow.schema.json @@ -70,6 +70,9 @@ "RefKind": { "type": "string" }, + "OptionalRef": { + "type": "boolean" + }, "ShowOnCard": { "type": "boolean" }, diff --git a/internal/domain/composition/builtinworkflows_codingloop.go b/internal/domain/composition/builtinworkflows_codingloop.go index b1ebafff3..ed5a48a3c 100644 --- a/internal/domain/composition/builtinworkflows_codingloop.go +++ b/internal/domain/composition/builtinworkflows_codingloop.go @@ -45,7 +45,7 @@ func codingLoopBuiltInWorkflow() []Workflow { {ID: "coding-loop-e2", Source: applyID, Target: notifyID}, }, BuiltIn: true, - Seed: seedorigin.Stamp(1), + Seed: seedorigin.Stamp(2), }, } } diff --git a/internal/domain/composition/codeexec.go b/internal/domain/composition/codeexec.go index 03f5ffe42..dfb0ddd21 100644 --- a/internal/domain/composition/codeexec.go +++ b/internal/domain/composition/codeexec.go @@ -39,6 +39,10 @@ type ResolvedExecEnv struct { ProfileMode string Dir string Env []string + // Label is the environment's own display name -- the shell step's + // target line names which environment a block runs inside + // (docs/goals/0240 S4); code-execution ignores it. + Label string } // lookupExecEnvFn defaults to erroring so a code-execution node run diff --git a/internal/domain/composition/codingloopsecrets_test.go b/internal/domain/composition/codingloopsecrets_test.go index 75423c4ba..9574f4a98 100644 --- a/internal/domain/composition/codingloopsecrets_test.go +++ b/internal/domain/composition/codingloopsecrets_test.go @@ -156,6 +156,7 @@ func TestProcessShellCommandExec_SecretChain_NoRefsLeavesEnvNil(t *testing.T) { env, redactValues := resolveShellSecretEnv( []ParsedCommandStep{{Index: 0, Text: "echo hello"}}, ExecContext{}, + nil, ) if env != nil { t.Errorf("env = %v, want nil (no secret placeholder referenced)", env) diff --git a/internal/domain/composition/executeshellcommand.go b/internal/domain/composition/executeshellcommand.go index ab55a078d..77e9174ce 100644 --- a/internal/domain/composition/executeshellcommand.go +++ b/internal/domain/composition/executeshellcommand.go @@ -40,6 +40,57 @@ import ( type ResolvedShellCommandTarget struct { Shell string Dir string + // EnvLabel names the Configure-authored execution environment the + // block runs inside (docs/goals/0240 S4) -- empty in the default + // real-login-shell posture. + EnvLabel string + // env is the child process's exact environment when an ExecEnv is + // set (explicit-only, codeexec.go's materialize-don't-inherit + // posture); nil in the default posture, where the process inherits + // the caller's real environment. + env []string + // argvFor builds the spawn argv for one sub-command -- the ExecEnv + // path routes through codeexec.go's shellArgv (clean/login flags + // per the environment's ProfileMode); the default path is the bare + // login-shell `-c` invocation this node has always used. + argvFor func(script string) []string +} + +// resolveShellCommandRunTarget picks the block's execution target from +// the node's own envId config (docs/goals/0240 S4): empty keeps the +// documented default posture (the user's real login shell and real +// environment, exactly as S1 shipped); a set envId resolves the +// Configure-authored ExecEnv through the SAME lookup code-execution +// uses -- shell flags via shellArgv, a per-BLOCK materialized dir via +// resolveDir (one temp dir for the whole block, so its sub-commands +// see each other's files), and the environment's explicit Env with +// codeexec.go's same minimal-PATH default when it declares none. +func resolveShellCommandRunTarget(envID string, run SecretAccessRun) (ResolvedShellCommandTarget, error) { + if strings.TrimSpace(envID) == "" { + t := ResolveShellCommandTarget() + t.argvFor = func(script string) []string { return []string{t.Shell, "-c", script} } + return t, nil + } + re, err := lookupExecEnvFn(envID, run) + if err != nil { + return ResolvedShellCommandTarget{}, fmt.Errorf("process-shell-command: %w", err) + } + dir, err := resolveDir(re.Dir) + if err != nil { + return ResolvedShellCommandTarget{}, fmt.Errorf("process-shell-command: %w", err) + } + env := re.Env + if len(env) == 0 { + env = []string{"PATH=/usr/bin:/bin:/usr/sbin:/sbin"} + } + shell, profile := re.Shell, re.ProfileMode + return ResolvedShellCommandTarget{ + Shell: shellArgv(shell, profile, "")[0], + Dir: dir, + EnvLabel: re.Label, + env: env, + argvFor: func(script string) []string { return shellArgv(shell, profile, script) }, + }, nil } // ResolveShellCommandTarget reads the process's own SHELL/HOME, exactly @@ -141,13 +192,20 @@ type shellStepOutcome struct { // "verbatim except the resolved secret" contract), and redactValues is // every non-empty resolved value regardless of source, for the output // redaction pass below to scrub. -func resolveShellSecretEnv(steps []ParsedCommandStep, ctx ExecContext) (env []string, redactValues []string) { +func resolveShellSecretEnv(steps []ParsedCommandStep, ctx ExecContext, baseEnv []string) (env []string, redactValues []string) { names := ExtractSecretEnvRefsAll(steps) if len(names) == 0 { - return nil, nil + // An ExecEnv target's environment stays explicit-only even with + // no secret refs to resolve (docs/goals/0240 S4) -- baseEnv nil + // is the default posture's inherit-the-real-environment case. + return baseEnv, nil } run := secretAccessRunFromCtx(ctx) - env = append(env, os.Environ()...) + if baseEnv != nil { + env = append(env, baseEnv...) + } else { + env = append(env, os.Environ()...) + } for _, name := range names { value, _, found := shellSecretResolverFn(name, ctx.SecretsToken, run) if !found { @@ -198,7 +256,7 @@ func runShellStep(node Node, step ParsedCommandStep, total int, target ResolvedS } handle, err := startShellProcessFn(procexec.Spec{ - Argv: []string{target.Shell, "-c", step.Text}, + Argv: target.argvFor(step.Text), Dir: target.Dir, // Env nil (the common case, resolveShellSecretEnv's own doc // comment) falls back to the calling process's real environment @@ -248,8 +306,11 @@ func runShellStep(node Node, step ParsedCommandStep, total int, target ResolvedS // states. steps is always non-empty (the init() closure below rejects // an empty parse before calling this). func runShellCommandBlock(node Node, ctx ExecContext, steps []ParsedCommandStep) (ExecContext, error) { - target := ResolveShellCommandTarget() - env, redactValues := resolveShellSecretEnv(steps, ctx) + target, err := resolveShellCommandRunTarget(node.Config["envId"], secretAccessRunFromCtx(ctx)) + if err != nil { + return ctx, err + } + env, redactValues := resolveShellSecretEnv(steps, ctx, target.env) var combined strings.Builder // lastFailed propagates a failure forward ONLY across && steps // (docs/goals/0240 S1: "&&"'s own short-circuit meaning, preserved @@ -299,7 +360,14 @@ func init() { Produces: PayloadProduce{Kind: PayloadText}, Output: "combined stdout+stderr from every sub-command that ran", Label: "Run a captured command", - Description: "Runs the captured payload in your real login shell, exactly as written -- no sandboxing, no stored environment profile. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default. No configurable fields: this node always runs whatever payload it receives.", + Description: "Runs the captured payload exactly as written -- in your real login shell by default, or inside a Configure-authored execution environment (its shell, directory, and variables) when one is chosen. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default.", + ConfigFields: []ConfigField{ + { + Key: "envId", Label: "Execution environment", + Description: "Runs the block inside a Configure-authored environment. Empty runs your real login shell.", + Default: "", Type: FieldText, RefKind: "execenv", OptionalRef: true, + }, + }, }, func(node Node, ctx ExecContext) (ExecContext, error) { steps := ParseShellCommandBlock(ctx.Payload) if len(steps) == 0 { diff --git a/internal/domain/composition/executeshellcommand_execenv_test.go b/internal/domain/composition/executeshellcommand_execenv_test.go new file mode 100644 index 000000000..297aa0fce --- /dev/null +++ b/internal/domain/composition/executeshellcommand_execenv_test.go @@ -0,0 +1,96 @@ +package composition + +import ( + "reflect" + "testing" +) + +// The shell step's optional execution-environment target +// (docs/goals/0240 S4): an envId routes the block through the SAME +// ExecEnv machinery code-execution uses -- shellArgv's clean/login +// flags, resolveDir, explicit-only env with the minimal-PATH default +// -- while an empty envId keeps S1's documented posture (the user's +// real login shell and real environment) byte-for-byte. + +func withExecEnvLookup(t *testing.T, re ResolvedExecEnv) { + t.Helper() + orig := lookupExecEnvFn + lookupExecEnvFn = func(envID string, _ SecretAccessRun) (ResolvedExecEnv, error) { return re, nil } + t.Cleanup(func() { lookupExecEnvFn = orig }) +} + +func TestResolveShellCommandRunTarget_EnvSet_UsesExecEnvMachinery(t *testing.T) { + dir := t.TempDir() + withExecEnvLookup(t, ResolvedExecEnv{ + Shell: "sh", ProfileMode: "clean", Dir: dir, + Env: []string{"FOO=bar"}, Label: "Safe sandbox", + }) + + target, err := resolveShellCommandRunTarget("some-env", SecretAccessRun{}) + if err != nil { + t.Fatalf("resolveShellCommandRunTarget: %v", err) + } + if target.EnvLabel != "Safe sandbox" { + t.Errorf("EnvLabel = %q, want the environment's own label", target.EnvLabel) + } + if target.Dir != dir { + t.Errorf("Dir = %q, want the environment's dir %q", target.Dir, dir) + } + if !reflect.DeepEqual(target.env, []string{"FOO=bar"}) { + t.Errorf("env = %v, want the environment's explicit env", target.env) + } + // The spawn argv carries sh's clean invocation, not a bare `-c`. + if got := target.argvFor("echo hi"); !reflect.DeepEqual(got, []string{"/bin/sh", "-c", "echo hi"}) { + t.Errorf("argvFor = %v, want sh clean-mode argv", got) + } +} + +func TestResolveShellCommandRunTarget_EnvWithEmptyEnv_GetsMinimalPATH(t *testing.T) { + dir := t.TempDir() + withExecEnvLookup(t, ResolvedExecEnv{Shell: "zsh", ProfileMode: "login", Dir: dir, Label: "L"}) + + target, err := resolveShellCommandRunTarget("some-env", SecretAccessRun{}) + if err != nil { + t.Fatalf("resolveShellCommandRunTarget: %v", err) + } + if !reflect.DeepEqual(target.env, []string{"PATH=/usr/bin:/bin:/usr/sbin:/sbin"}) { + t.Errorf("env = %v, want the minimal PATH default (explicit-only, never os.Environ)", target.env) + } + if got := target.argvFor("pwd"); !reflect.DeepEqual(got, []string{"/bin/zsh", "-l", "-c", "pwd"}) { + t.Errorf("argvFor = %v, want zsh login-mode argv", got) + } +} + +func TestResolveShellCommandRunTarget_EmptyEnvID_KeepsDefaultPosture(t *testing.T) { + target, err := resolveShellCommandRunTarget("", SecretAccessRun{}) + if err != nil { + t.Fatalf("resolveShellCommandRunTarget: %v", err) + } + if target.EnvLabel != "" || target.env != nil { + t.Errorf("target = %+v, want no env label and a nil (inherit-the-real-environment) env", target) + } + base := ResolveShellCommandTarget() + if got := target.argvFor("echo hi"); !reflect.DeepEqual(got, []string{base.Shell, "-c", "echo hi"}) { + t.Errorf("argvFor = %v, want the bare login-shell -c invocation", got) + } +} + +// A block-referenced secret upserts onto the ENVIRONMENT's explicit +// env, never onto os.Environ -- the two postures must not mix. +func TestResolveShellSecretEnv_ExplicitBaseStaysExplicit(t *testing.T) { + origResolver := shellSecretResolverFn + shellSecretResolverFn = func(varName string, _ string, _ SecretAccessRun) (string, SecretSource, bool) { + return "resolved-value", SecretSourceVault, true + } + t.Cleanup(func() { shellSecretResolverFn = origResolver }) + + steps := []ParsedCommandStep{{Index: 0, Text: "curl -H \"Authorization: $API_TOKEN\" https://x.test"}} + env, redact := resolveShellSecretEnv(steps, ExecContext{}, []string{"FOO=bar"}) + want := []string{"FOO=bar", "API_TOKEN=resolved-value"} + if !reflect.DeepEqual(env, want) { + t.Errorf("env = %v, want the explicit base plus the upserted secret (never os.Environ)", env) + } + if !reflect.DeepEqual(redact, []string{"resolved-value"}) { + t.Errorf("redact = %v, want the resolved value", redact) + } +} diff --git a/internal/domain/composition/graphpreflight.go b/internal/domain/composition/graphpreflight.go index 2f87e54c9..ae13397bc 100644 --- a/internal/domain/composition/graphpreflight.go +++ b/internal/domain/composition/graphpreflight.go @@ -53,7 +53,7 @@ func validateRequiredRefs(nodes []Node) []Issue { continue } for _, field := range nt.ConfigFields { - if field.RefKind == "" { + if field.RefKind == "" || field.OptionalRef { continue } if strings.TrimSpace(n.Config[field.Key]) != "" { diff --git a/internal/domain/typedfield/typedfield.go b/internal/domain/typedfield/typedfield.go index d00253254..9d35299a4 100644 --- a/internal/domain/typedfield/typedfield.go +++ b/internal/domain/typedfield/typedfield.go @@ -125,6 +125,14 @@ type Field struct { // field. Orthogonal to Type: the wire value is still a plain string // ID. RefKind string + // OptionalRef marks a RefKind field whose EMPTY value is a legal, + // meaningful configuration rather than "not configured yet" + // (docs/goals/0240 S4's shell-step environment: empty means the + // user's real login shell, a deliberate posture, not a gap) -- + // graph preflight skips its required-ref check. Meaningless + // without RefKind. JSON-tagged omitempty for the same + // byte-identical-marshaling reason Deprecated below documents. + OptionalRef bool `json:"OptionalRef,omitempty"` // ShowOnCard surfaces this field's value on a card's compact face // (docs/goals/0152) -- read by Atlas Kinds only; other // schema-carrying surfaces (List columns, Decision outputs) ignore diff --git a/internal/services/codeloopsvc/codeloopservice.go b/internal/services/codeloopsvc/codeloopservice.go index 15f1c50ae..0934e80b6 100644 --- a/internal/services/codeloopsvc/codeloopservice.go +++ b/internal/services/codeloopsvc/codeloopservice.go @@ -45,6 +45,11 @@ type CommandBlockPreview struct { Steps []CommandBlockPreviewStep `json:"steps"` Shell string `json:"shell"` Dir string `json:"dir"` + // EnvironmentLabel names the Configure-authored execution + // environment the seeded shell step is configured to run inside + // (docs/goals/0240 S4) -- empty in the default real-login-shell + // posture, where Shell/Dir above already tell the whole story. + EnvironmentLabel string `json:"environmentLabel"` // GuardrailVerdict is "allow" | "ask" | "deny" -- the block-level // gate decision (goal 0240 S3): the MOST RESTRICTIVE of every step's // own Verdict below, since the block still checkpoints/pauses as ONE @@ -92,6 +97,8 @@ type CodeLoopService struct { guard *guardrailsvc.GuardrailService exec *executionsvc.ExecutionService typedSecrets typedSecretsStore + // shellEnvPreviewFn -- see SetShellEnvPreview. + shellEnvPreviewFn func() (label, shell, dir string, ok bool) } // NewCodeLoopService constructs the service -- guard is read-only here @@ -112,6 +119,19 @@ func (s *CodeLoopService) SetExecutionService(exec *executionsvc.ExecutionServic s.exec = exec } +// SetShellEnvPreview wires the Confirm screen's environment lookup +// (docs/goals/0240 S4): reads the seeded shell step's CURRENT envId +// and that environment's label/shell/dir WITHOUT resolving its env +// values -- a preview must never trigger vault reads or their audit +// lines, so this deliberately does not ride lookupExecEnvFn. Wired +// from the composition root; nil (never wired, or no environment set) +// keeps the default real-login-shell preview. +// +//wails:ignore +func (s *CodeLoopService) SetShellEnvPreview(fn func() (label, shell, dir string, ok bool)) { + s.shellEnvPreviewFn = fn +} + // PreviewCommandBlock parses text and returns the Confirm screen's full // preview. Returns an error only for an empty/whitespace-only block -- // every other shape (a placeholder-looking secret, a step the guardrail @@ -143,6 +163,12 @@ func (s *CodeLoopService) PreviewCommandBlock(text string) (CommandBlockPreview, } target := composition.ResolveShellCommandTarget() + envLabel := "" + if s.shellEnvPreviewFn != nil { + if label, shell, dir, ok := s.shellEnvPreviewFn(); ok { + envLabel, target.Shell, target.Dir = label, shell, dir + } + } verdict := guardrail.WorstVerdict(stepVerdicts) names := composition.ExtractSecretEnvRefsAll(parsed) @@ -154,6 +180,7 @@ func (s *CodeLoopService) PreviewCommandBlock(text string) (CommandBlockPreview, return CommandBlockPreview{ Steps: steps, Shell: target.Shell, Dir: target.Dir, + EnvironmentLabel: envLabel, GuardrailVerdict: string(verdict.Effect), WorkflowID: composition.CodingLoopWorkflowID, NodeID: composition.CodingLoopShellStepID, diff --git a/internal/services/codeloopsvc/codeloopservice_shellguard_test.go b/internal/services/codeloopsvc/codeloopservice_shellguard_test.go index 33c1ead83..c24f9aa3a 100644 --- a/internal/services/codeloopsvc/codeloopservice_shellguard_test.go +++ b/internal/services/codeloopsvc/codeloopservice_shellguard_test.go @@ -89,3 +89,27 @@ func TestPreviewCommandBlock_DenyListedLine_TopLevelVerdictIsAsk(t *testing.T) { t.Errorf("Steps[0] = %+v, want ask with a non-empty rule label naming the deny-list rule", preview.Steps[0]) } } + +// The Confirm screen names the configured execution environment +// (docs/goals/0240 S4): a wired preview lookup overrides shell/dir and +// sets the label; unwired (or no environment set) keeps the default +// real-login-shell target untouched. +func TestPreviewCommandBlock_EnvironmentLabel(t *testing.T) { + s := newTestPreviewService(t) + preview, err := s.PreviewCommandBlock("echo hi") + if err != nil { + t.Fatalf("PreviewCommandBlock: %v", err) + } + if preview.EnvironmentLabel != "" { + t.Fatalf("unwired EnvironmentLabel = %q, want empty", preview.EnvironmentLabel) + } + + s.SetShellEnvPreview(func() (string, string, string, bool) { return "Safe sandbox", "/bin/sh", "/tmp/box", true }) + preview, err = s.PreviewCommandBlock("echo hi") + if err != nil { + t.Fatalf("PreviewCommandBlock (env): %v", err) + } + if preview.EnvironmentLabel != "Safe sandbox" || preview.Shell != "/bin/sh" || preview.Dir != "/tmp/box" { + t.Fatalf("preview = %+v, want the environment's label/shell/dir", preview) + } +} diff --git a/internal/services/configuresvc/configureexecenv.go b/internal/services/configuresvc/configureexecenv.go index 3eae54f03..6e1e15b69 100644 --- a/internal/services/configuresvc/configureexecenv.go +++ b/internal/services/configuresvc/configureexecenv.go @@ -90,7 +90,7 @@ func (c *ConfigureService) resolveExecEnv(id string, run composition.SecretAcces return composition.ResolvedExecEnv{}, err } return composition.ResolvedExecEnv{ - Shell: string(found.Shell), ProfileMode: string(found.ProfileMode), Dir: found.Dir, Env: env, + Shell: string(found.Shell), ProfileMode: string(found.ProfileMode), Dir: found.Dir, Env: env, Label: found.Label, }, nil } diff --git a/internal/services/seeding/seed_fingerprints.json b/internal/services/seeding/seed_fingerprints.json index a9d91a1c4..450777bfd 100644 --- a/internal/services/seeding/seed_fingerprints.json +++ b/internal/services/seeding/seed_fingerprints.json @@ -196,8 +196,8 @@ "fingerprint": "cbd789a974956ceea36271f5240de1d118b6966e4a7c9637fb00467f07f03d00" }, "workflow:coding-loop-run-copied-command-workflow": { - "seedRevision": 1, - "fingerprint": "318bcade8496af80091696c776202693cfa0a435d168cdb9fc8a9f47602e852e" + "seedRevision": 2, + "fingerprint": "ebf1af11ee05a917b9a32c405acf2f1648bca1727d01f8764f4d41ac5c68bef6" }, "workflow:example-ai-classify-branch-workflow": { "seedRevision": 2, diff --git a/internal/services/wiring/wiring.go b/internal/services/wiring/wiring.go index 9250ac0d6..9a3bbe321 100644 --- a/internal/services/wiring/wiring.go +++ b/internal/services/wiring/wiring.go @@ -12,6 +12,7 @@ import ( "log" "log/slog" "os" + "strings" "time" "github.com/alicoding/mill/internal/adapters/buildinfo" @@ -413,3 +414,40 @@ func WireWorkflowLifecycle(comp *compositionsvc.CompositionService, triggers *tr } triggers.PruneOrphanedHotkeys(ids) } + +// WireCodingLoopEnvPreview connects the Confirm screen's environment +// line (docs/goals/0240 S4): the seeded shell step's CURRENT envId +// (read live from CompositionService, so an edit takes effect on the +// next preview) joined to that environment's label/shell/dir from +// ConfigureService's plain entity list -- deliberately NOT the +// secret-resolving lookup, since a preview must never trigger vault +// reads or their audit lines. +func WireCodingLoopEnvPreview(codeLoop *codeloopsvc.CodeLoopService, comp *compositionsvc.CompositionService, cfg *configuresvc.ConfigureService) { + codeLoop.SetShellEnvPreview(func() (string, string, string, bool) { + envID := codingLoopShellEnvID(comp) + if envID == "" { + return "", "", "", false + } + for _, e := range cfg.ExecEnvs() { + if e.ID == envID { + return e.Label, string(e.Shell), e.Dir, true + } + } + return "", "", "", false + }) +} + +// codingLoopShellEnvID reads the seeded shell step's current envId. +func codingLoopShellEnvID(comp *compositionsvc.CompositionService) string { + for _, wf := range comp.Workflows() { + if wf.ID != composition.CodingLoopWorkflowID { + continue + } + for _, n := range wf.Nodes { + if n.ID == composition.CodingLoopShellStepID { + return strings.TrimSpace(n.Config["envId"]) + } + } + } + return "" +} diff --git a/main.go b/main.go index 870a79f9d..ee69900a6 100644 --- a/main.go +++ b/main.go @@ -203,6 +203,7 @@ func main() { } codeLoopService.SetExecutionService(executionService) wiring.WireCodingLoopSecrets(codeLoopService, secretService) + wiring.WireCodingLoopEnvPreview(codeLoopService, compositionService, configureService) // docs/goals/0240 S4 // Single execution path (docs/adr/0008): a headless trigger fire now // runs through the same durable ExecutionService.RunWorkflow every // other entrypoint uses, tagged RunKindTriggered -- constructed after diff --git a/userdocs/llms-full.txt b/userdocs/llms-full.txt index ede96d3df..0d67bc302 100644 --- a/userdocs/llms-full.txt +++ b/userdocs/llms-full.txt @@ -850,10 +850,12 @@ Looks up an Attributes value in a configured List and writes the matched entry b ### Run a captured command -Runs the captured payload in your real login shell, exactly as written -- no sandboxing, no stored environment profile. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default. No configurable fields: this node always runs whatever payload it receives. +Runs the captured payload exactly as written -- in your real login shell by default, or inside a Configure-authored execution environment (its shell, directory, and variables) when one is chosen. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default. - Takes: text — Produces: text - Effect: external — parks for approval by default +- Settings: + - **Execution environment** — Runs the block inside a Configure-authored environment. Empty runs your real login shell. (references an Execution environment) ### Run a command diff --git a/userdocs/reference/steps.md b/userdocs/reference/steps.md index 90bb0a2c2..1e3fae563 100644 --- a/userdocs/reference/steps.md +++ b/userdocs/reference/steps.md @@ -239,10 +239,12 @@ Looks up an Attributes value in a configured List and writes the matched entry b ### Run a captured command -Runs the captured payload in your real login shell, exactly as written -- no sandboxing, no stored environment profile. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default. No configurable fields: this node always runs whatever payload it receives. +Runs the captured payload exactly as written -- in your real login shell by default, or inside a Configure-authored execution environment (its shell, directory, and variables) when one is chosen. A piped command stays one step; commands separated by a new line or && show as separate steps. External effect -- the run asks for your approval by default. - Takes: text — Produces: text - Effect: external — parks for approval by default +- Settings: + - **Execution environment** — Runs the block inside a Configure-authored environment. Empty runs your real login shell. (references an Execution environment) ### Run a command