diff --git a/README.md b/README.md index d7a78d350..41a1452b8 100644 --- a/README.md +++ b/README.md @@ -101,8 +101,8 @@ gog --account you@gmail.com \ ``` See [Automation](docs/automation.md) for output and exit-code contracts, and -[Safety Profiles](docs/safety-profiles.md) for binaries with command policy -baked in at build time. +[Safety Profiles](docs/safety-profiles.md) for binaries with command policy and +locked flag values baked in at build time. ## Accounts and authentication diff --git a/cmd/bake-safety-profile/locked_flags_test.go b/cmd/bake-safety-profile/locked_flags_test.go new file mode 100644 index 000000000..44fbee0c5 --- /dev/null +++ b/cmd/bake-safety-profile/locked_flags_test.go @@ -0,0 +1,54 @@ +package main + +import ( + "bytes" + "fmt" + "go/parser" + "go/token" + "testing" + + "github.com/openclaw/gogcli/internal/safetyprofile" +) + +func TestGenerateLockedFlagsEmitsHashedLookup(t *testing.T) { + profile := &safetyprofile.Profile{ + Name: "test", + AllowRules: []string{"gmail.get"}, + LockedFlags: []safetyprofile.LockedFlag{ + {Name: "sanitize-content", Value: "true"}, + {Name: "inline-max-bytes", Value: "8388608"}, + }, + } + + out := generate(profile) + + if _, err := parser.ParseFile(token.NewFileSet(), "gen.go", out, parser.AllErrors); err != nil { + t.Fatalf("generated code does not parse as Go:\n%s\n\nerror: %v", out, err) + } + + // The flag name must be hashed like a command rule, and the value emitted as the + // literal the flag parser consumes. + for _, flag := range profile.LockedFlags { + want := fmt.Sprintf("\tcase 0x%016x:\n\t\treturn %q, true\n", safetyprofile.HashRule(flag.Name), flag.Value) + if !bytes.Contains(out, []byte(want)) { + t.Fatalf("generated output missing case for %q:\n%s\n\nfull output:\n%s", flag.Name, want, out) + } + if bytes.Contains(out, []byte(`"`+flag.Name+`"`)) { + t.Fatalf("locked flag name %q appears verbatim; it should only be hashed\n\nfull output:\n%s", flag.Name, out) + } + } +} + +// A profile with no locked flags still needs the lookup, so safety_profile builds +// compile whether or not the profile uses the feature. +func TestGenerateWithoutLockedFlagsStillDefinesLookup(t *testing.T) { + out := generate(&safetyprofile.Profile{Name: "test", AllowRules: []string{"gmail.get"}}) + + if _, err := parser.ParseFile(token.NewFileSet(), "gen.go", out, parser.AllErrors); err != nil { + t.Fatalf("generated code does not parse as Go:\n%s\n\nerror: %v", out, err) + } + want := "func bakedSafetyLockedFlag(name string) (string, bool) {\n\treturn \"\", false\n}\n" + if !bytes.Contains(out, []byte(want)) { + t.Fatalf("generated output missing the empty lookup:\n%s", out) + } +} diff --git a/cmd/bake-safety-profile/main.go b/cmd/bake-safety-profile/main.go index c4194ff62..294a818d2 100644 --- a/cmd/bake-safety-profile/main.go +++ b/cmd/bake-safety-profile/main.go @@ -64,10 +64,38 @@ func generate(profile *safetyprofile.Profile) []byte { writeMatcher(&out, "bakedSafetyAllowMatch", profile.AllowRules, profile.AllowAll) out.WriteString("\n") writeMatcher(&out, "bakedSafetyDenyMatch", profile.DenyRules, false) + out.WriteString("\n") + writeLockedFlags(&out, profile.LockedFlags) return out.Bytes() } +// writeLockedFlags emits the locked flag lookup. Names are hashed like the command +// rules; the locked values are literals because the flag parser consumes them. The +// count travels alongside so the runtime can tell that every lock matched a real +// flag without the names being written down. +func writeLockedFlags(out *bytes.Buffer, flags []safetyprofile.LockedFlag) { + out.WriteString("func bakedSafetyLockedFlag(name string) (string, bool) {\n") + if len(flags) == 0 { + out.WriteString("\treturn \"\", false\n}\n") + out.WriteString("\nfunc bakedSafetyLockedFlagCount() int { return 0 }\n") + return + } + out.WriteString("\tswitch bakedSafetyHashPath([]string{name}) {\n") + seen := make(map[uint64]string, len(flags)) + for _, flag := range flags { + h := safetyprofile.HashRule(flag.Name) + if existing, dup := seen[h]; dup { + fmt.Fprintf(os.Stderr, "bake-safety-profile: hash collision between locked flags %q and %q (FNV-64a=%#x)\n", existing, flag.Name, h) + os.Exit(1) + } + seen[h] = flag.Name + fmt.Fprintf(out, "\tcase 0x%016x:\n\t\treturn %s, true\n", h, strconv.Quote(flag.Value)) + } + out.WriteString("\t}\n\treturn \"\", false\n}\n") + fmt.Fprintf(out, "\nfunc bakedSafetyLockedFlagCount() int { return %d }\n", len(flags)) +} + func writeMatcher(out *bytes.Buffer, name string, rules []string, matchAll bool) { fmt.Fprintf(out, "func %s(path []string) bool {\n", name) if matchAll { diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 0e8981436..76b53d685 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -9,6 +9,10 @@ Runtime guards such as `--enable-commands`, `--disable-commands`, and stronger: the policy is compiled into the binary and cannot be changed with flags, environment variables, config files, or shell arguments. +A profile controls which commands may run. Through `locked-flags` it can also fix +the value of individual flags, so settings such as sanitized output do not depend +on the caller passing them. See [Locked Flags](#locked-flags). + ## Quick Start Build an agent-safe binary: @@ -111,6 +115,20 @@ Good for: - monitoring - read-only agent context gathering +`safety-profiles/agent-safe-locked.yaml` and `safety-profiles/readonly-locked.yaml` + +The same command rules as `agent-safe` and `readonly`, plus locked +`sanitize-content`, `wrap-untrusted`, and `no-input`. The read-only one also locks +`readonly`, which rejects a mutating request made inside an allowed command. Note +that locking `sanitize-content` makes `gmail get --format raw` unavailable, since +that combination is rejected. + +Good for: + +- agents that must not be able to request unsanitized or unmarked output +- unattended jobs where a prompt would hang the caller +- handing a binary to a caller whose command line you do not control + `safety-profiles/full.yaml` Allows everything. This is mostly useful for smoke testing the build path or for @@ -142,6 +160,7 @@ Rules: - unlisted commands are blocked when the profile has any allow rules. - command names are written as dot paths internally, such as `gmail.drafts.create`. - `aliases:` controls root shortcuts such as `send`, `ls`, `search`, and `upload`. +- `locked-flags:` is not a command path; it locks flag values, see [Locked Flags](#locked-flags). Parent rules are prefix matches. For example, `drive: true` allows every `drive` subcommand unless a child is explicitly blocked. For restrictive profiles, prefer @@ -155,6 +174,83 @@ gmail: modify: false ``` +## Locked Flags + +Command rules decide what may run. They do not decide how it runs. Sanitized +output, for example, happens only when the caller passes `--sanitize-content`, +and a flag that does take its value from the environment is still overridden by +the command line. When the command line is written by a model rather than a +person, neither is a setting you can rely on. + +`locked-flags` fixes a flag's value in the profile: + +```yaml +locked-flags: + sanitize-content: true + wrap-untrusted: true + no-input: true + inline-max-bytes: 8388608 +``` + +Values may be booleans, integers, or strings, and are parsed exactly as the flag +itself parses them. + +A locked flag behaves as follows: + +- the value is applied before the command runs, so the caller need not pass it. +- setting that flag on the command line is an error, not an override. +- aliases are covered, since the check is on the canonical flag name. +- a locked flag that the selected command does not define is ignored rather than + an error, so per-command flags can be locked without breaking other commands. +- a locked output mode wins over the competing one. With `json` locked, `GOG_PLAIN` + gives way silently, since an environment default is a preference rather than a + request, while an explicit `--plain` is refused for asking output the profile + forbids. +- commands that build partial requests from which flags were given treat a locked + flag as given, so the locked value reaches the request. + +Two kinds of lock are refused rather than half-applied, because their value is +consumed before locks run: `--home`, which the layout resolver reads straight from +argv to choose config and credential roots, and any flag marked required, which Kong +validates during parsing. A name matching no flag at all is refused too. In each case +the binary declines to run instead of reporting a guarantee it does not have. + +Setting a locked flag fails before the command handler runs: + +```text +flag --sanitize-content is locked by baked safety profile "agent-safe-locked" +``` + +Because a locked value can make a command reject a combination the caller never +asked for, usage errors name the locked flags: + +```text +--sanitize-content cannot be used with --format raw +note: --no-input, --sanitize-content, --wrap-untrusted locked by baked safety profile "agent-safe-locked" +``` + +Lock deliberately. A flag that participates in a conflict or a requirement removes +the command paths that contradict it. Locking `sanitize-content` makes +`gmail get --format raw` unavailable, which is an acceptable trade for an agent +profile because raw is the unsanitized dump. + +Locking a flag that requires another one is the case to avoid. `--reply-all` needs +a message or thread to reply to, so `locked-flags: {reply-all: true}` breaks an +ordinary draft: + +```bash +bin/gog-my-agent gmail drafts create --to you@example.com --subject Hi --body Hi +``` + +The command is allowed by the profile and the caller passed nothing wrong, yet it +fails because the locked flag demands an argument they had no reason to supply: + +```text +--reply-all requires --reply-to-message-id or --thread-id +``` + +Flags that only shape output are the safe candidates. + ## Choosing A Profile Use `readonly` when the caller should never change Google or local `gog` state. diff --git a/internal/cmd/kong_helpers.go b/internal/cmd/kong_helpers.go index 77c238510..c074e3c6f 100644 --- a/internal/cmd/kong_helpers.go +++ b/internal/cmd/kong_helpers.go @@ -2,7 +2,18 @@ package cmd import "github.com/alecthomas/kong" +// flagProvided reports whether a command should treat the flag as supplied. A value +// a baked profile locked counts: commands that build partial requests from which +// flags were given would otherwise drop a locked value on the floor, leaving the +// lock set on the struct but absent from the request. func flagProvided(kctx *kong.Context, name string) bool { + return flagOnCommandLine(kctx, name) || lockedFlagNames[name] +} + +// flagOnCommandLine reports only what the caller typed. Lock enforcement and +// output-mode precedence need that narrower question: one to detect an override +// attempt, the other to rank an explicit flag against an environment default. +func flagOnCommandLine(kctx *kong.Context, name string) bool { if kctx == nil { return false } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 60a8a2f11..e984a2834 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -165,7 +165,6 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { cli.diagnostics = runtimeIO.Err cli.authOperations = runtime.Auth cli.authMode = googleapi.ParseAuthMode(os.Getenv("GOG_AUTH_MODE")) - applyExplicitOutputModePrecedence(kctx, &cli.RootFlags) // Make config-backed account and alias resolution available to the // pre-Run enforcement hooks below (enforceGmailNoSend resolves the @@ -182,6 +181,17 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { if err = enforceBakedSafetyProfile(kctx); err != nil { return reportEarlyError(runtimeIO.Err, err) } + if err = verifyLockedFlagsExist(kctx); err != nil { + return reportEarlyError(runtimeIO.Err, err) + } + if err = enforceLockedFlags(kctx); err != nil { + return reportEarlyError(runtimeIO.Err, err) + } + // After the locks, so a locked output mode is what precedence resolves around + // rather than something a competing mode can leave in conflict. + if err = applyExplicitOutputModePrecedence(kctx, &cli.RootFlags); err != nil { + return reportEarlyError(runtimeIO.Err, err) + } if err = enforceEnabledCommands(kctx, cli.EnableCommands, cli.EnableCommandsExact); err != nil { return reportEarlyError(runtimeIO.Err, err) } @@ -334,13 +344,13 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { err = stableExitCode(err) if u := ui.FromContext(ctx); u != nil { - msg := strings.TrimSpace(errfmt.Format(err)) + msg := errorMessage(err) if msg != "" { u.Err().Error(msg) } return err } - msg := strings.TrimSpace(errfmt.Format(err)) + msg := errorMessage(err) if msg != "" { _, _ = fmt.Fprintln(runtimeIO.Err, msg) } @@ -400,19 +410,37 @@ func validateJSONTransformFlags(mode outfmt.Mode, flags *RootFlags) error { } } -func applyExplicitOutputModePrecedence(kctx *kong.Context, flags *RootFlags) { +// applyExplicitOutputModePrecedence settles --json against --plain, which cannot +// both be set. A locked mode outranks the competing one: typing that competing flag +// is refused the way setting the locked flag itself is, since the caller asked for +// output the profile forbids, while an environment default gives way silently +// because it is an ambient setting rather than a request about this invocation. +func applyExplicitOutputModePrecedence(kctx *kong.Context, flags *RootFlags) error { if flags == nil { - return + return nil } - jsonSet := flagProvided(kctx, "json") - plainSet := flagProvided(kctx, "plain") + jsonLocked := lockedFlagNames["json"] + plainLocked := lockedFlagNames["plain"] + jsonSet := flagOnCommandLine(kctx, "json") + plainSet := flagOnCommandLine(kctx, "plain") switch { + case jsonLocked && !plainLocked: + if plainSet { + return usagef("flag --plain conflicts with --json, locked by baked safety profile %q", bakedSafetyProfileName()) + } + flags.Plain = false + case plainLocked && !jsonLocked: + if jsonSet { + return usagef("flag --json conflicts with --plain, locked by baked safety profile %q", bakedSafetyProfileName()) + } + flags.JSON = false case jsonSet && !plainSet: flags.Plain = false case plainSet && !jsonSet: flags.JSON = false } + return nil } func reportEarlyError(w io.Writer, err error) error { @@ -426,6 +454,22 @@ func reportEarlyError(w io.Writer, err error) error { return err } +// errorMessage renders a command's error for display. Usage errors carry the +// locked-flag note, because a baked profile can supply a value the caller never +// passed and the rejection then names a flag absent from their command line. The +// pre-run enforcement errors skip it: those name the locked flag themselves. +func errorMessage(err error) string { + msg := strings.TrimSpace(errfmt.Format(err)) + if msg == "" || ExitCode(err) != 2 { + return msg + } + note := lockedFlagsNote() + if note == "" { + return msg + } + return msg + "\n" + note +} + func isTerminalWriter(w io.Writer) bool { file, ok := w.(*os.File) return ok && termutil.IsTerminal(file) diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 64132bd80..6f2f12af2 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -1,7 +1,9 @@ package cmd import ( + "fmt" "hash/fnv" + "sort" "strings" "github.com/alecthomas/kong" @@ -43,6 +45,104 @@ func enforceBakedSafetyProfile(kctx *kong.Context) error { return nil } +// lockedFlagNames records the flags enforceLockedFlags set, so a command rejecting a +// value it never received on the command line can say where that value came from. +var lockedFlagNames = map[string]bool{} + +// lockedFlagsNote names the locked flags for display beneath a usage error. A command +// can reject a combination involving a value the caller never passed, so the note is +// what explains where that value came from. Empty when nothing is locked. +func lockedFlagsNote() string { + if len(lockedFlagNames) == 0 { + return "" + } + names := make([]string, 0, len(lockedFlagNames)) + for name := range lockedFlagNames { + names = append(names, "--"+name) + } + sort.Strings(names) + return fmt.Sprintf("note: %s locked by baked safety profile %q", strings.Join(names, ", "), bakedSafetyProfileName()) +} + +// verifyLockedFlagsExist refuses to run when a locked name matches no flag in the +// CLI. Enforcement only ever asks whether a given flag is locked, so a misspelled +// name would lock nothing at all and the profile would claim a guarantee it does not +// have. Counting the matches catches that without the names appearing in the binary. +func verifyLockedFlagsExist(kctx *kong.Context) error { + want := bakedSafetyLockedFlagCount() + if want == 0 { + return nil + } + seen := map[string]bool{} + var unsupported error + var walk func(node *kong.Node) + walk = func(node *kong.Node) { + if node == nil { + return + } + for _, flag := range node.Flags { + if _, locked := bakedSafetyLockedFlag(flag.Name); !locked { + continue + } + seen[flag.Name] = true + if err := lockUnsupported(flag); err != nil && unsupported == nil { + unsupported = err + } + } + for _, child := range node.Children { + walk(child) + } + } + walk(kctx.Model.Node) + if len(seen) < want { + return usagef("baked safety profile %q locks %d flag(s) but only %d exist; check the locked-flags names", bakedSafetyProfileName(), want, len(seen)) + } + return unsupported +} + +// lockUnsupported names the flags a lock cannot reach, because their value is +// consumed before locks are applied. Refusing them is honest: the alternative is a +// profile that reports a guarantee the run never had. +func lockUnsupported(flag *kong.Flag) error { + switch { + case flag.Name == "home": + // The layout resolver reads --home straight from argv before Kong parses, so + // config and credential roots are already chosen by the time locks run. + return usagef("baked safety profile %q locks --home, which is read before flags are parsed and cannot be locked", bakedSafetyProfileName()) + case flag.Required: + // Kong rejects a missing required flag during parsing, so a locked one fails + // when omitted and is refused as an override when supplied. + return usagef("baked safety profile %q locks required flag --%s, which must be supplied on the command line", bakedSafetyProfileName(), flag.Name) + } + return nil +} + +// enforceLockedFlags applies the profile's locked flag values and refuses a command +// line that sets one of them. The value is locked rather than merely defaulted so it +// holds without help from the environment, which the caller may not control. +func enforceLockedFlags(kctx *kong.Context) error { + // Rebuilt per parse: carrying names over would let one run's note describe a + // profile that is not in force. + lockedFlagNames = map[string]bool{} + if !bakedSafetyEnabled() { + return nil + } + for _, flag := range kctx.Flags() { + value, locked := bakedSafetyLockedFlag(flag.Name) + if !locked { + continue + } + if flagOnCommandLine(kctx, flag.Name) { + return usagef("flag --%s is locked by baked safety profile %q", flag.Name, bakedSafetyProfileName()) + } + if err := flag.Value.Parse(kong.ScanFromTokens(kong.Token{Type: kong.FlagValueToken, Value: value}), flag.Value.Target); err != nil { + return usagef("locked flag --%s: %v", flag.Name, err) + } + lockedFlagNames[flag.Name] = true + } + return nil +} + func bakedSafetyProfileError(path []string, profileName string, included bool) error { command := strings.Join(path, " ") if included { diff --git a/internal/cmd/safety_profile_default.go b/internal/cmd/safety_profile_default.go index 5cf14ef66..78eaea1fc 100644 --- a/internal/cmd/safety_profile_default.go +++ b/internal/cmd/safety_profile_default.go @@ -16,6 +16,7 @@ var bakedSafetyTestProfile struct { allowAll bool allow map[string]bool deny map[string]bool + lockedFlags map[string]string } func bakedSafetyEnabled() bool { return bakedSafetyTestProfile.enabled } @@ -32,3 +33,12 @@ func bakedSafetyAllowMatch(path []string) bool { func bakedSafetyDenyMatch(path []string) bool { return commandPathMatches(bakedSafetyTestProfile.deny, path) } + +func bakedSafetyLockedFlag(name string) (string, bool) { + value, ok := bakedSafetyTestProfile.lockedFlags[name] + return value, ok +} + +func bakedSafetyLockedFlagCount() int { + return len(bakedSafetyTestProfile.lockedFlags) +} diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go new file mode 100644 index 000000000..0967a5b43 --- /dev/null +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -0,0 +1,317 @@ +package cmd + +import ( + "strings" + "testing" +) + +const lockedFlagsProfile = ` +name: locked +locked-flags: + sanitize-content: true +gmail: + get: true + search: true +` + +// A lock must hold whatever spelling reaches it, including the value it already +// has: accepting a matching value would make the lock depend on what the caller +// asked for. +func TestLockedFlag_RejectsEveryFormOfSettingIt(t *testing.T) { + for _, arg := range []string{ + "--sanitize-content=false", + "--sanitize-content=true", + "--sanitize-content", + "--sanitize=false", + "--safe", + } { + t.Run(arg, func(t *testing.T) { + withBakedSafetyProfile(t, lockedFlagsProfile) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", arg, + }, nil) + if result.err == nil { + t.Fatalf("%s must be rejected, got stdout=%q", arg, result.stdout) + } + if !strings.Contains(result.err.Error(), "--sanitize-content is locked") { + t.Fatalf("%s: err = %v", arg, result.err) + } + }) + } +} + +// Locking a flag one command declares must not disturb commands that do not, or +// locking a per-command flag would break the rest of the CLI. +func TestLockedFlag_InertOnCommandsWithoutTheFlag(t *testing.T) { + withBakedSafetyProfile(t, lockedFlagsProfile) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "search", "newer_than:1d", + }, nil) + if result.err != nil && strings.Contains(result.err.Error(), "is locked") { + t.Fatalf("lock leaked into a command without the flag: %v", result.err) + } +} + +// Stock builds compile the no-profile stub, so locks must not apply at all. +func TestLockedFlag_IgnoredWithoutBakedProfile(t *testing.T) { + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", "--sanitize-content=false", + }, nil) + if result.err != nil && strings.Contains(result.err.Error(), "is locked") { + t.Fatalf("lock applied without a baked profile: %v", result.err) + } +} + +// A value the flag cannot parse must fail loudly; silently skipping it would leave +// the flag unlocked while the profile claims otherwise. +func TestLockedFlag_UnparsableValueFails(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + sanitize-content: notabool +gmail: + get: true +`) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", + }, nil) + if result.err == nil { + t.Fatalf("unparsable locked value must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "locked flag --sanitize-content") { + t.Fatalf("err = %v", result.err) + } +} + +// Non-bool locks go through the same flag parser, so prove an int actually lands. +func TestLockedFlag_NonBoolValueReachesCommand(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + inline-max-bytes: -1 +gmail: + attachment: true +`) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "attachment", "m1", "a1", + }, nil) + if result.err == nil { + t.Fatalf("locked inline-max-bytes must reach the command, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "--inline-max-bytes must be non-negative") { + t.Fatalf("err = %v", result.err) + } +} + +func TestLockedFlag_RejectsCommandLineOverride(t *testing.T) { + withBakedSafetyProfile(t, lockedFlagsProfile) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", "--sanitize-content=false", + }, nil) + if result.err == nil { + t.Fatalf("locked flag override must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "--sanitize-content is locked") { + t.Fatalf("err = %v", result.err) + } +} + +func TestLockedFlag_RejectsAliasOverride(t *testing.T) { + withBakedSafetyProfile(t, lockedFlagsProfile) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", "--safe=false", + }, nil) + if result.err == nil { + t.Fatalf("alias override must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "--sanitize-content is locked") { + t.Fatalf("err = %v", result.err) + } +} + +// The locked value must reach the command, not merely block overrides. gmail get +// refuses --format raw when sanitize is on, so that refusal proves the profile set +// the flag even though the command line never mentioned it. +func TestLockedFlag_LockedValueReachesCommand(t *testing.T) { + withBakedSafetyProfile(t, lockedFlagsProfile) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", "--format", "raw", + }, nil) + if result.err == nil { + t.Fatalf("locked sanitize-content must reject --format raw, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "cannot be used with --format raw") { + t.Fatalf("err = %v", result.err) + } + // The caller never passed the flag, so the printed message has to say where the + // value came from. The note is added at display time, not to the error itself. + if !strings.Contains(result.stderr, `note: --sanitize-content locked by baked safety profile "locked"`) { + t.Fatalf("stderr lacks the locked-flag note: %q", result.stderr) + } +} + +// Without the profile the same command line is accepted, so the rejection above +// comes from the lock rather than from the flag being unusable. +func TestLockedFlag_UnlockedProfileAllowsOverride(t *testing.T) { + withBakedSafetyProfile(t, "name: unlocked\ngmail:\n get: true\n") + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", "--sanitize-content=false", + }, nil) + if result.err != nil && strings.Contains(result.err.Error(), "is locked") { + t.Fatalf("unlocked profile must not reject the flag: %v", result.err) + } +} + +const lockedJSONProfile = ` +name: locked +locked-flags: + json: true +version: true +` + +// --json and --plain are mutually exclusive, so asking for the competing mode is +// asking for output the profile forbids. Refusing it rather than quietly producing +// the locked mode keeps typing --plain as visible as typing the locked flag itself. +func TestLockedFlag_LockedJSONRejectsExplicitPlain(t *testing.T) { + withBakedSafetyProfile(t, lockedJSONProfile) + result := executeWithTestRuntime(t, []string{"--plain", "version"}, nil) + if result.err == nil { + t.Fatalf("--plain against a locked json must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), `--plain conflicts with --json, locked by baked safety profile "locked"`) { + t.Fatalf("err = %v", result.err) + } +} + +func TestLockedFlag_LockedJSONBeatsEnvironmentPlain(t *testing.T) { + t.Setenv("GOG_PLAIN", "1") + withBakedSafetyProfile(t, lockedJSONProfile) + result := executeWithTestRuntime(t, []string{"version"}, nil) + if result.err != nil { + t.Fatalf("locked json with GOG_PLAIN must not fail: %v (stderr=%q)", result.err, result.stderr) + } + if !strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { + t.Fatalf("expected JSON output, got %q", result.stdout) + } +} + +func TestLockedFlag_LockedPlainRejectsExplicitJSON(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + plain: true +version: true +`) + result := executeWithTestRuntime(t, []string{"--json", "version"}, nil) + if result.err == nil { + t.Fatalf("--json against a locked plain must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), `--json conflicts with --plain, locked by baked safety profile "locked"`) { + t.Fatalf("err = %v", result.err) + } +} + +// The locked mode still applies when nothing competes on the command line. +func TestLockedFlag_LockedPlainAppliesWithoutCompetingFlag(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + plain: true +version: true +`) + result := executeWithTestRuntime(t, []string{"version"}, nil) + if result.err != nil { + t.Fatalf("locked plain must apply: %v (stderr=%q)", result.err, result.stderr) + } + if strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { + t.Fatalf("expected plain output, got %q", result.stdout) + } +} + +// A locked name that matches no flag locks nothing, so the binary must refuse to run +// rather than report a guarantee it cannot keep. +func TestLockedFlag_NonexistentNameRefusesToRun(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + sanitize-contnet: true +gmail: + get: true +`) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "get", "m1", + }, nil) + if result.err == nil { + t.Fatalf("a misspelled locked flag must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "locks 1 flag(s) but only 0 exist") { + t.Fatalf("err = %v", result.err) + } +} + +// Commands that build partial requests ask which flags were given, so a locked value +// has to count as given or it never reaches the request. Lock enforcement asks the +// narrower question and must not see itself as an override. +func TestLockedFlag_CountsAsProvidedButNotAsTyped(t *testing.T) { + previous := lockedFlagNames + lockedFlagNames = map[string]bool{"summary": true} + t.Cleanup(func() { lockedFlagNames = previous }) + + if !flagProvided(nil, "summary") { + t.Fatal("a locked flag must count as provided") + } + if flagOnCommandLine(nil, "summary") { + t.Fatal("a locked flag must not count as typed on the command line") + } +} + +// --home is read from argv before Kong parses, so a lock on it would leave the +// caller's own config and credential roots in place while the profile claimed +// otherwise. Refusing is the honest outcome. +func TestLockedFlag_HomeIsRefused(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + home: /tmp/locked-home +version: true +`) + result := executeWithTestRuntime(t, []string{"version"}, nil) + if result.err == nil { + t.Fatalf("locking --home must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "locks --home") { + t.Fatalf("err = %v", result.err) + } +} + +// Kong validates required flags before locks run, so a locked required flag can +// never be satisfied: omitted it fails parsing, supplied it is refused as an override. +func TestLockedFlag_RequiredFlagIsRefused(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + to: someone@example.com +gmail: + send: true +`) + result := executeWithTestRuntime(t, []string{ + "--json", "--account", "a@b.com", + "gmail", "send", "--subject", "x", "--body", "y", + }, nil) + if result.err == nil { + t.Fatalf("locking a required flag must fail, got stdout=%q", result.stdout) + } + if !strings.Contains(result.err.Error(), "locks required flag") { + t.Fatalf("err = %v", result.err) + } +} diff --git a/internal/cmd/safety_profile_test.go b/internal/cmd/safety_profile_test.go index d3ba212c7..db63709a7 100644 --- a/internal/cmd/safety_profile_test.go +++ b/internal/cmd/safety_profile_test.go @@ -33,6 +33,11 @@ func withBakedSafetyProfile(t *testing.T, raw string) { bakedSafetyTestProfile.hasAllowRules = profile.AllowAll || len(profile.AllowRules) > 0 bakedSafetyTestProfile.allow = allow bakedSafetyTestProfile.deny = deny + locked := make(map[string]string, len(profile.LockedFlags)) + for _, f := range profile.LockedFlags { + locked[f.Name] = f.Value + } + bakedSafetyTestProfile.lockedFlags = locked t.Cleanup(func() { bakedSafetyTestProfile = prev }) } diff --git a/internal/safetyprofile/locked_flags_test.go b/internal/safetyprofile/locked_flags_test.go new file mode 100644 index 000000000..cd494de3d --- /dev/null +++ b/internal/safetyprofile/locked_flags_test.go @@ -0,0 +1,65 @@ +package safetyprofile + +import "testing" + +func TestParse_LockedFlagsValues(t *testing.T) { + profile, err := Parse(` +name: locked +locked-flags: + sanitize-content: true + inline-max-bytes: 8388608 + format: metadata +gmail: + get: true +`) + if err != nil { + t.Fatalf("Parse: %v", err) + } + want := []LockedFlag{ + {Name: "format", Value: "metadata"}, + {Name: "inline-max-bytes", Value: "8388608"}, + {Name: "sanitize-content", Value: "true"}, + } + if len(profile.LockedFlags) != len(want) { + t.Fatalf("locked flags = %#v", profile.LockedFlags) + } + for i, w := range want { + if profile.LockedFlags[i] != w { + t.Fatalf("locked flag %d = %#v, want %#v", i, profile.LockedFlags[i], w) + } + } +} + +func TestParse_LockedFlagsRejectsNonScalar(t *testing.T) { + _, err := Parse(` +name: locked +locked-flags: + sanitize-content: + nested: true +gmail: + get: true +`) + if err == nil { + t.Fatal("nested locked-flags value must be rejected") + } +} + +// locked-flags must not be flattened into command rules the way other top-level +// keys are. +func TestParse_LockedFlagsAreNotCommandRules(t *testing.T) { + profile, err := Parse(` +name: locked +locked-flags: + sanitize-content: true +gmail: + get: true +`) + if err != nil { + t.Fatalf("Parse: %v", err) + } + for _, rule := range append(profile.AllowRules, profile.DenyRules...) { + if rule == "locked-flags.sanitize-content" || rule == "sanitize-content" { + t.Fatalf("locked flag leaked into command rules: %v", rule) + } + } +} diff --git a/internal/safetyprofile/parse.go b/internal/safetyprofile/parse.go index c431805e0..6c83dc7e1 100644 --- a/internal/safetyprofile/parse.go +++ b/internal/safetyprofile/parse.go @@ -3,6 +3,7 @@ package safetyprofile import ( "fmt" "sort" + "strconv" "strings" "go.yaml.in/yaml/v3" @@ -25,6 +26,10 @@ func Parse(raw string) (*Profile, error) { Name: parsed.name, AllowAll: parsed.allow[literalAll] || parsed.allow["*"], } + for name, value := range parsed.lockedFlags { + out.LockedFlags = append(out.LockedFlags, LockedFlag{Name: name, Value: value}) + } + sort.Slice(out.LockedFlags, func(i, j int) bool { return out.LockedFlags[i].Name < out.LockedFlags[j].Name }) for k := range parsed.allow { if k == literalAll || k == "*" { continue @@ -40,9 +45,10 @@ func Parse(raw string) (*Profile, error) { } type rawProfile struct { - name string - allow map[string]bool - deny map[string]bool + name string + allow map[string]bool + deny map[string]bool + lockedFlags map[string]string } func parseRaw(raw string) (*rawProfile, error) { @@ -52,9 +58,10 @@ func parseRaw(raw string) (*rawProfile, error) { } profile := &rawProfile{ - name: "unnamed", - allow: map[string]bool{}, - deny: map[string]bool{}, + name: "unnamed", + allow: map[string]bool{}, + deny: map[string]bool{}, + lockedFlags: map[string]string{}, } if rawName, present := root["name"]; present { @@ -70,10 +77,13 @@ func parseRaw(raw string) (*rawProfile, error) { if err := addList(profile.deny, root["deny"]); err != nil { return nil, fmt.Errorf("deny: %w", err) } + if err := addLockedFlags(profile.lockedFlags, root["locked-flags"]); err != nil { + return nil, fmt.Errorf("locked-flags: %w", err) + } for key, value := range root { switch key { - case "name", "description", "allow", "deny": + case "name", "description", "allow", "deny", "locked-flags": continue } prefix := []string{key} @@ -116,6 +126,35 @@ func addList(out map[string]bool, value any) error { return nil } +// addLockedFlags reads the locked-flags mapping of flag name to locked value. Values +// are rendered as the literal a flag parses, matching how Kong reads an envar. +func addLockedFlags(out map[string]string, value any) error { + if value == nil { + return nil + } + entries, ok := value.(map[string]any) + if !ok { + return fmt.Errorf("expected mapping of flag name to value") + } + for name, raw := range entries { + flag := strings.TrimSpace(strings.ToLower(name)) + if flag == "" { + return fmt.Errorf("empty flag name") + } + switch typed := raw.(type) { + case bool: + out[flag] = strconv.FormatBool(typed) + case int: + out[flag] = strconv.Itoa(typed) + case string: + out[flag] = typed + default: + return fmt.Errorf("%s: expected bool, int or string value, got %T", flag, raw) + } + } + return nil +} + func flatten(profile *rawProfile, prefix []string, value any) error { switch typed := value.(type) { case bool: diff --git a/internal/safetyprofile/profile.go b/internal/safetyprofile/profile.go index f8b770290..f1acc7f05 100644 --- a/internal/safetyprofile/profile.go +++ b/internal/safetyprofile/profile.go @@ -14,4 +14,14 @@ type Profile struct { AllowAll bool AllowRules []string DenyRules []string + // LockedFlags fix a flag to a value the command line cannot change. Sorted by + // name so the generated switch is stable across builds. + LockedFlags []LockedFlag +} + +// LockedFlag is one locked flag. Value is the literal the flag parses, in the same +// form an environment variable would supply. +type LockedFlag struct { + Name string + Value string } diff --git a/safety-profiles/agent-safe-locked.yaml b/safety-profiles/agent-safe-locked.yaml new file mode 100644 index 000000000..8422f1844 --- /dev/null +++ b/safety-profiles/agent-safe-locked.yaml @@ -0,0 +1,357 @@ +name: agent-safe-locked +description: agent-safe plus locked output flags. Same command rules; sanitized, marker-wrapped, non-interactive output that the command line cannot switch off. Kept separate so agent-safe keeps its current behaviour. Note that locking sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. + +# Locked for every invocation. Setting one of these on the command line is an error +# rather than an override, so a model driving this binary cannot ask for unsanitized +# or unmarked output, nor leave the process waiting on a prompt nobody will answer. +locked-flags: + sanitize-content: true + wrap-untrusted: true + no-input: true + +version: true +schema: true + +gmail: + search: true + get: true + messages: true + attachment: true + url: true + history: true + thread: + get: true + modify: true + attachments: true + labels: + list: true + get: true + create: true + rename: true + modify: true + delete: false + style: true + batch: + modify: true + delete: false + archive: true + mark-read: true + unread: true + trash: false + send: false + import: false + autoreply: false + track: false + drafts: + list: true + get: true + create: true + update: true + delete: false + send: false + settings: false + watch: false + autoforward: false + delegates: false + filters: false + forwarding: false + sendas: false + vacation: false + forward: false + +calendar: + calendars: true + subscribe: false + acl: true + alias: true + events: true + event: true + create: true + update: true + move: true + delete: false + freebusy: true + respond: false + propose-time: true + colors: true + conflicts: true + search: true + time: true + users: true + team: true + focus-time: true + out-of-office: false + working-location: false + create-calendar: true + +drive: + ls: true + search: true + get: true + download: true + upload: true + mkdir: true + copy: true + delete: false + move: true + rename: true + share: false + unshare: false + permissions: true + url: true + drives: true + comments: + list: true + get: true + create: true + update: true + delete: false + reply: true + +sites: + list: true + search: true + get: true + url: true + +contacts: + search: true + list: true + get: true + export: true + create: false + update: false + delete: false + directory: + list: true + search: true + other: + list: true + search: true + delete: false + +tasks: + lists: + list: true + create: true + list: true + get: true + add: true + update: true + done: true + undo: true + delete: false + clear: false + +docs: + export: true + info: true + cat: true + list-tabs: true + suggestions: + list: true + create: false + copy: false + write: false + insert: false + delete: false + find-replace: false + update: false + edit: false + sed: false + clear: false + table-row: + insert: false + delete: false + table-column: + insert: false + delete: false + table-merge: false + table-unmerge: false + structure: true + named-range: + list: true + create: false + delete: false + replace: false + comments: + list: true + get: true + add: true + reply: true + resolve: false + delete: false + +sheets: + get: true + metadata: true + notes: true + update-note: false + links: true + validation: + get: true + set: false + clear: false + named-ranges: + list: true + get: true + add: false + update: false + delete: false + read-format: true + export: true + update: false + batch-update: false + append: false + insert: false + delete-dimension: false + clear: false + format: false + merge: false + unmerge: false + number-format: false + freeze: false + resize-columns: false + resize-rows: false + find-replace: false + create: false + copy: false + add-tab: true + rename-tab: true + delete-tab: false + chart: + list: true + get: true + create: true + update: true + delete: false + +slides: + export: true + info: true + list-slides: true + read-slide: true + thumbnail: true + create: false + create-from-markdown: false + create-from-template: false + copy: false + add-slide: false + delete-slide: false + update-notes: false + replace-slide: false + insert-text: false + replace-text: false + +chat: + spaces: + list: true + find: true + create: false + messages: + list: true + send: false + react: true + reactions: false + threads: + list: true + dm: + send: false + space: false + +forms: + get: true + create: false + update: false + publish: false + add-question: false + delete-question: false + move-question: true + responses: + list: true + get: true + watch: false + +appscript: + get: true + content: true + run: false + create: false + +people: + me: true + get: true + search: true + relations: true + +groups: + list: true + members: true + +keep: + list: true + get: true + search: true + create: true + delete: false + attachment: true + +auth: + credentials: + list: true + set: false + remove: false + services: true + list: true + doctor: true + alias: + list: true + set: false + unset: false + status: true + keyring: false + add: false + remove: false + tokens: + list: true + delete: false + export: false + import: false + manage: false + service-account: + status: true + set: false + unset: false + keep: false + +config: + get: true + keys: true + list: true + path: true + set: false + unset: false + no-send: + list: true + set: false + remove: false + +time: true +classroom: false +admin: false +backup: false +completion: false +__complete: false + +aliases: + send: false + ls: true + search: true + open: true + download: true + upload: true + login: false + logout: false + status: true + me: true + whoami: true diff --git a/safety-profiles/readonly-locked.yaml b/safety-profiles/readonly-locked.yaml new file mode 100644 index 000000000..dc8dd2f96 --- /dev/null +++ b/safety-profiles/readonly-locked.yaml @@ -0,0 +1,363 @@ +name: readonly-locked +description: readonly plus locked output flags. Same command rules; sanitized, marker-wrapped, non-interactive output and runtime read-only enforcement that the command line cannot switch off. Kept separate so readonly keeps its current behaviour. Note that locking sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. + +# Locked for every invocation. Setting one of these on the command line is an error +# rather than an override. readonly is locked here as well as denied per command, +# because it also rejects a mutating request made inside a command this profile allows. +locked-flags: + sanitize-content: true + wrap-untrusted: true + no-input: true + readonly: true + +version: true +schema: true + +gmail: + search: true + get: true + messages: + search: true + modify: false + attachment: true + url: true + history: true + thread: + get: true + modify: false + attachments: true + labels: + list: true + get: true + create: false + rename: false + modify: false + delete: false + style: false + batch: + modify: false + delete: false + archive: false + mark-read: false + unread: false + trash: false + send: false + import: false + autoreply: false + track: false + drafts: + list: true + get: true + create: false + update: false + delete: false + send: false + settings: false + watch: false + autoforward: false + delegates: false + filters: false + forwarding: false + sendas: false + vacation: false + forward: false + +calendar: + calendars: true + subscribe: false + acl: true + alias: + list: true + set: false + unset: false + events: true + event: true + create: false + update: false + move: false + delete: false + freebusy: true + respond: false + propose-time: false + colors: true + conflicts: true + search: true + time: true + users: true + team: true + focus-time: false + out-of-office: false + working-location: false + create-calendar: false + +drive: + ls: true + search: true + get: true + download: true + upload: false + mkdir: false + copy: false + delete: false + move: false + rename: false + share: false + unshare: false + permissions: true + url: true + drives: true + comments: + list: true + get: true + create: false + update: false + delete: false + reply: false + +sites: + list: true + search: true + get: true + url: true + +contacts: + search: true + list: true + get: true + export: true + create: false + update: false + delete: false + directory: + list: true + search: true + other: + list: true + search: true + delete: false + +tasks: + lists: + list: true + create: false + list: true + get: true + add: false + update: false + done: false + undo: false + delete: false + clear: false + +docs: + export: true + info: true + cat: true + list-tabs: true + suggestions: + list: true + create: false + copy: false + write: false + insert: false + delete: false + find-replace: false + update: false + edit: false + sed: false + clear: false + table-row: + insert: false + delete: false + table-column: + insert: false + delete: false + table-merge: false + table-unmerge: false + structure: true + named-range: + list: true + create: false + delete: false + replace: false + comments: + list: true + get: true + add: false + reply: false + resolve: false + delete: false + +sheets: + get: true + metadata: true + notes: true + update-note: false + links: true + validation: + get: true + set: false + clear: false + named-ranges: + list: true + get: true + add: false + update: false + delete: false + read-format: true + export: true + update: false + batch-update: false + append: false + insert: false + delete-dimension: false + clear: false + format: false + merge: false + unmerge: false + number-format: false + freeze: false + resize-columns: false + resize-rows: false + find-replace: false + create: false + copy: false + add-tab: false + rename-tab: false + delete-tab: false + chart: + list: true + get: true + create: false + update: false + delete: false + +slides: + export: true + info: true + list-slides: true + read-slide: true + thumbnail: true + create: false + create-from-markdown: false + create-from-template: false + copy: false + add-slide: false + delete-slide: false + update-notes: false + replace-slide: false + insert-text: false + replace-text: false + +chat: + spaces: + list: true + find: true + create: false + messages: + list: true + send: false + react: false + reactions: false + threads: + list: true + dm: + send: false + space: false + +forms: + get: true + create: false + update: false + publish: false + add-question: false + delete-question: false + move-question: false + responses: + list: true + get: true + watch: false + +appscript: + get: true + content: true + run: false + create: false + +people: + me: true + get: true + search: true + relations: true + +groups: + list: true + members: true + +keep: + list: true + get: true + search: true + create: false + delete: false + attachment: true + +auth: + credentials: + list: true + set: false + remove: false + services: true + list: true + doctor: true + alias: + list: true + set: false + unset: false + status: true + keyring: false + add: false + remove: false + tokens: + list: true + delete: false + export: false + import: false + manage: false + service-account: + status: true + set: false + unset: false + keep: false + +config: + get: true + keys: true + list: true + path: true + set: false + unset: false + no-send: + list: true + set: false + remove: false + +time: true +classroom: false +admin: false +backup: false +completion: false +__complete: false + +aliases: + send: false + ls: true + search: true + open: true + download: true + upload: false + login: false + logout: false + status: true + me: true + whoami: true