From 69b851baa89df7dd7bd2c0e52ad7ed3347277f5b Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 15:00:22 +0200 Subject: [PATCH 01/12] feat(safety-profile): pin flag values with locked-flags so a baked profile can fix output settings the command line cannot change --- cmd/bake-safety-profile/main.go | 24 ++ internal/cmd/root.go | 23 +- internal/cmd/safety_profile.go | 44 +++ internal/cmd/safety_profile_default.go | 6 + .../cmd/safety_profile_locked_flags_test.go | 77 ++++ internal/cmd/safety_profile_test.go | 5 + internal/safetyprofile/locked_flags_test.go | 65 ++++ internal/safetyprofile/parse.go | 53 ++- internal/safetyprofile/profile.go | 10 + safety-profiles/agent-safe-locked.yaml | 355 ++++++++++++++++++ 10 files changed, 653 insertions(+), 9 deletions(-) create mode 100644 internal/cmd/safety_profile_locked_flags_test.go create mode 100644 internal/safetyprofile/locked_flags_test.go create mode 100644 safety-profiles/agent-safe-locked.yaml diff --git a/cmd/bake-safety-profile/main.go b/cmd/bake-safety-profile/main.go index c4194ff62..e79f83f35 100644 --- a/cmd/bake-safety-profile/main.go +++ b/cmd/bake-safety-profile/main.go @@ -64,10 +64,34 @@ 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 pinned flag lookup. Names are hashed like the command +// rules; the pinned values are literals because the flag parser consumes them. +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") + 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") +} + 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/internal/cmd/root.go b/internal/cmd/root.go index 6891e1c85..6e86035d3 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -182,6 +182,9 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { if err = enforceBakedSafetyProfile(kctx); err != nil { return reportEarlyError(runtimeIO.Err, err) } + if err = enforceLockedFlags(kctx); err != nil { + return reportEarlyError(runtimeIO.Err, err) + } if err = enforceEnabledCommands(kctx, cli.EnableCommands, cli.EnableCommandsExact); err != nil { return reportEarlyError(runtimeIO.Err, err) } @@ -309,13 +312,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) } @@ -401,6 +404,22 @@ func reportEarlyError(w io.Writer, err error) error { return err } +// errorMessage renders a command's error for display. Usage errors carry the +// pinned-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 := pinnedFlagsNote() + 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..0d8388f81 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,48 @@ func enforceBakedSafetyProfile(kctx *kong.Context) error { return nil } +// enforceLockedFlags applies the profile's pinned flag values and refuses a command +// line that sets one of them. The value is pinned rather than merely defaulted so it +// holds without help from the environment, which the caller may not control. +// pinnedFlagNames 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 pinnedFlagNames = map[string]bool{} + +// pinnedFlagsNote names the pinned 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 pinned. +func pinnedFlagsNote() string { + if len(pinnedFlagNames) == 0 { + return "" + } + names := make([]string, 0, len(pinnedFlagNames)) + for name := range pinnedFlagNames { + names = append(names, "--"+name) + } + sort.Strings(names) + return fmt.Sprintf("note: %s pinned by baked safety profile %q", strings.Join(names, ", "), bakedSafetyProfileName()) +} + +func enforceLockedFlags(kctx *kong.Context) error { + if !bakedSafetyEnabled() { + return nil + } + for _, flag := range kctx.Flags() { + value, locked := bakedSafetyLockedFlag(flag.Name) + if !locked { + continue + } + if flagProvided(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) + } + pinnedFlagNames[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..4be432b62 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,8 @@ 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 +} 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..de0912e9c --- /dev/null +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -0,0 +1,77 @@ +package cmd + +import ( + "strings" + "testing" +) + +const lockedFlagsProfile = ` +name: locked +locked-flags: + sanitize-content: true +gmail: + get: true +` + +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 pinned 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_PinnedValueReachesCommand(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("pinned 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 pinned by baked safety profile "locked"`) { + t.Fatalf("stderr lacks the pinned-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) + } +} 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..30f3b9de6 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 pinned 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..ea8038621 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 pin 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 pinned 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..132ed9d80 --- /dev/null +++ b/safety-profiles/agent-safe-locked.yaml @@ -0,0 +1,355 @@ +name: agent-safe-locked +description: agent-safe plus pinned output flags. Same command rules; sanitized, marker-wrapped output that the command line cannot switch off. Kept as a separate profile so agent-safe keeps its current behaviour. + +# Pinned for every invocation. Setting one of these on the command line is an error +# rather than an override, so an agent cannot ask for unsanitized or unmarked output. +locked-flags: + sanitize-content: true + wrap-untrusted: 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 From a2baa265abe536413990f050b6be4a8941fcb484 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 16:34:29 +0200 Subject: [PATCH 02/12] feat(safety-profile): add readonly-locked and pin no-input, extending the locked variants to the read path --- safety-profiles/agent-safe-locked.yaml | 6 +- safety-profiles/readonly-locked.yaml | 363 +++++++++++++++++++++++++ 2 files changed, 367 insertions(+), 2 deletions(-) create mode 100644 safety-profiles/readonly-locked.yaml diff --git a/safety-profiles/agent-safe-locked.yaml b/safety-profiles/agent-safe-locked.yaml index 132ed9d80..3f36c341d 100644 --- a/safety-profiles/agent-safe-locked.yaml +++ b/safety-profiles/agent-safe-locked.yaml @@ -1,11 +1,13 @@ name: agent-safe-locked -description: agent-safe plus pinned output flags. Same command rules; sanitized, marker-wrapped output that the command line cannot switch off. Kept as a separate profile so agent-safe keeps its current behaviour. +description: agent-safe plus pinned 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 pinning sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. # Pinned for every invocation. Setting one of these on the command line is an error -# rather than an override, so an agent cannot ask for unsanitized or unmarked output. +# 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 diff --git a/safety-profiles/readonly-locked.yaml b/safety-profiles/readonly-locked.yaml new file mode 100644 index 000000000..6debfa590 --- /dev/null +++ b/safety-profiles/readonly-locked.yaml @@ -0,0 +1,363 @@ +name: readonly-locked +description: readonly plus pinned 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 pinning sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. + +# Pinned for every invocation. Setting one of these on the command line is an error +# rather than an override. readonly is pinned 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 From 7cabd50e71c11cbb3f295f863267e9210e9353e1 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 17:47:02 +0200 Subject: [PATCH 03/12] docs(safety-profiles): document locked-flags, the locked presets, and which flags not to lock --- README.md | 4 +- cmd/bake-safety-profile/main.go | 4 +- docs/safety-profiles.md | 79 +++++++++++++++++++ internal/cmd/root.go | 4 +- internal/cmd/safety_profile.go | 26 +++--- .../cmd/safety_profile_locked_flags_test.go | 10 +-- internal/safetyprofile/parse.go | 2 +- internal/safetyprofile/profile.go | 4 +- safety-profiles/agent-safe-locked.yaml | 4 +- safety-profiles/readonly-locked.yaml | 6 +- 10 files changed, 111 insertions(+), 32 deletions(-) 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/main.go b/cmd/bake-safety-profile/main.go index e79f83f35..b06a6bbd9 100644 --- a/cmd/bake-safety-profile/main.go +++ b/cmd/bake-safety-profile/main.go @@ -70,8 +70,8 @@ func generate(profile *safetyprofile.Profile) []byte { return out.Bytes() } -// writeLockedFlags emits the pinned flag lookup. Names are hashed like the command -// rules; the pinned values are literals because the flag parser consumes them. +// writeLockedFlags emits the locked flag lookup. Names are hashed like the command +// rules; the locked values are literals because the flag parser consumes them. func writeLockedFlags(out *bytes.Buffer, flags []safetyprofile.LockedFlag) { out.WriteString("func bakedSafetyLockedFlag(name string) (string, bool) {\n") if len(flags) == 0 { diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 0e8981436..4aae28f86 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, and — through `locked-flags` — may 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,15 @@ 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` — and `readonly` on the +read-only one, which also rejects a mutating request made inside an allowed +command. Use these when the caller is a model that would otherwise be free to ask +for unsanitized or unmarked output. Note that locking `sanitize-content` makes +`gmail get --format raw` unavailable, since that combination is rejected. + `safety-profiles/full.yaml` Allows everything. This is mostly useful for smoke testing the build path or for @@ -142,6 +155,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 +169,71 @@ gmail: modify: false ``` +## Locked Flags + +Command rules decide what may run. They cannot decide *how* it runs, so a setting +like sanitized output stays a default that the command line overrides. When the +command line is written by a model rather than a person, that distinction matters. + +`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: + +```text +flag --sanitize-content is locked by baked safety profile "agent-safe-locked" +``` + +- 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. + +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, because the +command rejects that combination — an acceptable trade for an agent profile, since +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 every +ordinary draft: + +```bash +bin/gog-my-agent gmail drafts create --to you@example.com --subject Hi --body Hi +``` +```text +--reply-all requires --reply-to-message-id or --thread-id +``` + +The command is allowed by the profile and the caller passed nothing wrong; it fails +because the locked flag demands an argument they had no reason to supply. The same +applies to `--markdown`, which requires `--replace` or `--append`, and to either half +of a mutually exclusive pair such as `--all` and `--occurrence`. + +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/root.go b/internal/cmd/root.go index 09fc0f539..c2346036d 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -430,7 +430,7 @@ func reportEarlyError(w io.Writer, err error) error { } // errorMessage renders a command's error for display. Usage errors carry the -// pinned-flag note, because a baked profile can supply a value the caller never +// 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 { @@ -438,7 +438,7 @@ func errorMessage(err error) string { if msg == "" || ExitCode(err) != 2 { return msg } - note := pinnedFlagsNote() + note := lockedFlagsNote() if note == "" { return msg } diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 0d8388f81..5108bc287 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -45,28 +45,28 @@ func enforceBakedSafetyProfile(kctx *kong.Context) error { return nil } -// enforceLockedFlags applies the profile's pinned flag values and refuses a command -// line that sets one of them. The value is pinned rather than merely defaulted so it -// holds without help from the environment, which the caller may not control. -// pinnedFlagNames records the flags enforceLockedFlags set, so a command rejecting a +// 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 pinnedFlagNames = map[string]bool{} +var lockedFlagNames = map[string]bool{} -// pinnedFlagsNote names the pinned flags for display beneath a usage error. A command +// 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 pinned. -func pinnedFlagsNote() string { - if len(pinnedFlagNames) == 0 { +// 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(pinnedFlagNames)) - for name := range pinnedFlagNames { + names := make([]string, 0, len(lockedFlagNames)) + for name := range lockedFlagNames { names = append(names, "--"+name) } sort.Strings(names) - return fmt.Sprintf("note: %s pinned by baked safety profile %q", strings.Join(names, ", "), bakedSafetyProfileName()) + return fmt.Sprintf("note: %s locked by baked safety profile %q", strings.Join(names, ", "), bakedSafetyProfileName()) } +// 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 { if !bakedSafetyEnabled() { return nil @@ -82,7 +82,7 @@ func enforceLockedFlags(kctx *kong.Context) error { 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) } - pinnedFlagNames[flag.Name] = true + lockedFlagNames[flag.Name] = true } return nil } diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index de0912e9c..ab62cf16c 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -41,25 +41,25 @@ func TestLockedFlag_RejectsAliasOverride(t *testing.T) { } } -// The pinned value must reach the command, not merely block overrides. gmail get +// 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_PinnedValueReachesCommand(t *testing.T) { +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("pinned sanitize-content must reject --format raw, got stdout=%q", result.stdout) + 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 pinned by baked safety profile "locked"`) { - t.Fatalf("stderr lacks the pinned-flag note: %q", result.stderr) + 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) } } diff --git a/internal/safetyprofile/parse.go b/internal/safetyprofile/parse.go index 30f3b9de6..6c83dc7e1 100644 --- a/internal/safetyprofile/parse.go +++ b/internal/safetyprofile/parse.go @@ -126,7 +126,7 @@ func addList(out map[string]bool, value any) error { return nil } -// addLockedFlags reads the locked-flags mapping of flag name to pinned value. Values +// 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 { diff --git a/internal/safetyprofile/profile.go b/internal/safetyprofile/profile.go index ea8038621..f1acc7f05 100644 --- a/internal/safetyprofile/profile.go +++ b/internal/safetyprofile/profile.go @@ -14,12 +14,12 @@ type Profile struct { AllowAll bool AllowRules []string DenyRules []string - // LockedFlags pin a flag to a value the command line cannot change. Sorted by + // 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 pinned flag. Value is the literal the flag parses, in the same +// 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 diff --git a/safety-profiles/agent-safe-locked.yaml b/safety-profiles/agent-safe-locked.yaml index 3f36c341d..8422f1844 100644 --- a/safety-profiles/agent-safe-locked.yaml +++ b/safety-profiles/agent-safe-locked.yaml @@ -1,7 +1,7 @@ name: agent-safe-locked -description: agent-safe plus pinned 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 pinning sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. +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. -# Pinned for every invocation. Setting one of these on the command line is an error +# 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: diff --git a/safety-profiles/readonly-locked.yaml b/safety-profiles/readonly-locked.yaml index 6debfa590..dc8dd2f96 100644 --- a/safety-profiles/readonly-locked.yaml +++ b/safety-profiles/readonly-locked.yaml @@ -1,8 +1,8 @@ name: readonly-locked -description: readonly plus pinned 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 pinning sanitize-content makes `gmail get --format raw` unavailable, since that combination is rejected. +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. -# Pinned for every invocation. Setting one of these on the command line is an error -# rather than an override. readonly is pinned here as well as denied per command, +# 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 From 438ad1a6464cb972116b58d6b0d81b68120078c2 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 18:07:37 +0200 Subject: [PATCH 04/12] docs(safety-profiles): match the file's existing style in the locked-flags section --- docs/safety-profiles.md | 55 ++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 4aae28f86..d2cdf5b26 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -9,9 +9,9 @@ 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, and — through `locked-flags` — may 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). +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 @@ -118,11 +118,16 @@ Good for: `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` — and `readonly` on the -read-only one, which also rejects a mutating request made inside an allowed -command. Use these when the caller is a model that would otherwise be free to ask -for unsanitized or unmarked output. Note that locking `sanitize-content` makes -`gmail get --format raw` unavailable, since that combination is rejected. +`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` @@ -171,7 +176,7 @@ gmail: ## Locked Flags -Command rules decide what may run. They cannot decide *how* it runs, so a setting +Command rules decide what may run. They do not decide how it runs, so a setting like sanitized output stays a default that the command line overrides. When the command line is written by a model rather than a person, that distinction matters. @@ -191,16 +196,17 @@ 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: +- 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. + +Setting a locked flag fails before the command handler runs: ```text flag --sanitize-content is locked by baked safety profile "agent-safe-locked" ``` -- 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. - Because a locked value can make a command reject a combination the caller never asked for, usage errors name the locked flags: @@ -210,28 +216,25 @@ note: --no-input, --sanitize-content, --wrap-untrusted locked by baked safety pr ``` 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, because the -command rejects that combination — an acceptable trade for an agent profile, since -raw is the unsanitized dump. +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 every +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 ``` -The command is allowed by the profile and the caller passed nothing wrong; it fails -because the locked flag demands an argument they had no reason to supply. The same -applies to `--markdown`, which requires `--replace` or `--append`, and to either half -of a mutually exclusive pair such as `--all` and `--occurrence`. - Flags that only shape output are the safe candidates. ## Choosing A Profile From 49ac92b4a2ce4792b8a898ae0bb1552999a19717 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 18:17:27 +0200 Subject: [PATCH 05/12] docs(safety-profiles): correct how an unlocked flag's value is actually determined --- docs/safety-profiles.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index d2cdf5b26..69215d588 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -176,9 +176,11 @@ gmail: ## Locked Flags -Command rules decide what may run. They do not decide how it runs, so a setting -like sanitized output stays a default that the command line overrides. When the -command line is written by a model rather than a person, that distinction matters. +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: From fee967bd12b52691d06dc81af5f600c93beecd67 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 18:23:43 +0200 Subject: [PATCH 06/12] test(safety-profile): cover locked-flag bypass shapes, inertness, generator output; rebuild locked names per parse --- cmd/bake-safety-profile/locked_flags_test.go | 54 +++++++++++ internal/cmd/safety_profile.go | 3 + .../cmd/safety_profile_locked_flags_test.go | 95 +++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 cmd/bake-safety-profile/locked_flags_test.go 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/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 5108bc287..bec7f102e 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -68,6 +68,9 @@ func lockedFlagsNote() string { // 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 } diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index ab62cf16c..136d1e9ea 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -11,8 +11,103 @@ 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{ From fd24ccc7969f70b27c45cedc6ddfb05a7e3ba5ad Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 19:52:39 +0200 Subject: [PATCH 07/12] fix(safety-profile): resolve output-mode precedence after locks so a locked json or plain wins over the competing mode --- docs/safety-profiles.md | 2 + internal/cmd/root.go | 13 ++++- .../cmd/safety_profile_locked_flags_test.go | 49 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 69215d588..fcb92adc6 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -202,6 +202,8 @@ A locked flag behaves as follows: - 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. Locking `json` makes `--plain` + and `GOG_PLAIN` give way instead of failing as a conflicting mode. Setting a locked flag fails before the command handler runs: diff --git a/internal/cmd/root.go b/internal/cmd/root.go index c2346036d..40e6534a5 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 @@ -185,6 +184,9 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { 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. + applyExplicitOutputModePrecedence(kctx, &cli.RootFlags) if err = enforceEnabledCommands(kctx, cli.EnableCommands, cli.EnableCommandsExact); err != nil { return reportEarlyError(runtimeIO.Err, err) } @@ -408,9 +410,18 @@ func applyExplicitOutputModePrecedence(kctx *kong.Context, flags *RootFlags) { return } + // A locked mode outranks one the caller passed or the environment defaulted: + // the profile fixed it, so the competing mode gives way instead of leaving both + // set for outfmt.FromFlags to reject. + jsonLocked := lockedFlagNames["json"] + plainLocked := lockedFlagNames["plain"] jsonSet := flagProvided(kctx, "json") plainSet := flagProvided(kctx, "plain") switch { + case jsonLocked && !plainLocked: + flags.Plain = false + case plainLocked && !jsonLocked: + flags.JSON = false case jsonSet && !plainSet: flags.Plain = false case plainSet && !jsonSet: diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index 136d1e9ea..4211c1e73 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -170,3 +170,52 @@ func TestLockedFlag_UnlockedProfileAllowsOverride(t *testing.T) { t.Fatalf("unlocked profile must not reject the flag: %v", result.err) } } + +const lockedJSONProfile = ` +name: locked +locked-flags: + json: true +version: true +` + +// A locked output mode has to survive a competing one. --json and --plain are +// mutually exclusive, and the loser is resolved before the command runs, so a lock +// that lost would surface as a rejected invocation rather than the profile's mode. +func TestLockedFlag_LockedJSONBeatsExplicitPlain(t *testing.T) { + withBakedSafetyProfile(t, lockedJSONProfile) + result := executeWithTestRuntime(t, []string{"--plain", "version"}, nil) + if result.err != nil { + t.Fatalf("locked json with --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_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_LockedPlainBeatsExplicitJSON(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("locked plain with --json must not fail: %v (stderr=%q)", result.err, result.stderr) + } + if strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { + t.Fatalf("expected plain output, got %q", result.stdout) + } +} From 22e8ed956367ef239b5667cb48e21feebc732050 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 20:01:52 +0200 Subject: [PATCH 08/12] fix(safety-profile): refuse an explicit competing output mode against a locked one, override only environment defaults --- docs/safety-profiles.md | 6 ++- internal/cmd/root.go | 23 +++++++++--- .../cmd/safety_profile_locked_flags_test.go | 37 ++++++++++++++----- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index fcb92adc6..3d7722c19 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -202,8 +202,10 @@ A locked flag behaves as follows: - 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. Locking `json` makes `--plain` - and `GOG_PLAIN` give way instead of failing as a conflicting mode. +- 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. Setting a locked flag fails before the command handler runs: diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 40e6534a5..616e446d8 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -186,7 +186,9 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { } // After the locks, so a locked output mode is what precedence resolves around // rather than something a competing mode can leave in conflict. - applyExplicitOutputModePrecedence(kctx, &cli.RootFlags) + 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) } @@ -405,28 +407,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 } - // A locked mode outranks one the caller passed or the environment defaulted: - // the profile fixed it, so the competing mode gives way instead of leaving both - // set for outfmt.FromFlags to reject. jsonLocked := lockedFlagNames["json"] plainLocked := lockedFlagNames["plain"] jsonSet := flagProvided(kctx, "json") plainSet := flagProvided(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 { diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index 4211c1e73..8c11e2466 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -178,17 +178,17 @@ locked-flags: version: true ` -// A locked output mode has to survive a competing one. --json and --plain are -// mutually exclusive, and the loser is resolved before the command runs, so a lock -// that lost would surface as a rejected invocation rather than the profile's mode. -func TestLockedFlag_LockedJSONBeatsExplicitPlain(t *testing.T) { +// --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("locked json with --plain must not fail: %v (stderr=%q)", result.err, result.stderr) + if result.err == nil { + t.Fatalf("--plain against a locked json must fail, got stdout=%q", result.stdout) } - if !strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { - t.Fatalf("expected JSON output, got %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) } } @@ -204,7 +204,7 @@ func TestLockedFlag_LockedJSONBeatsEnvironmentPlain(t *testing.T) { } } -func TestLockedFlag_LockedPlainBeatsExplicitJSON(t *testing.T) { +func TestLockedFlag_LockedPlainRejectsExplicitJSON(t *testing.T) { withBakedSafetyProfile(t, ` name: locked locked-flags: @@ -212,8 +212,25 @@ locked-flags: 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 with --json must not fail: %v (stderr=%q)", result.err, result.stderr) + 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) From b3a42d56a2b3a1b6cd4855202b3c7af926f8c048 Mon Sep 17 00:00:00 2001 From: Ronny Rentner Date: Mon, 10 Aug 2026 20:58:08 +0200 Subject: [PATCH 09/12] fix(safety-profile): make locked flags count as provided, refuse names that match nothing, and refuse locks on --home or required flags --- cmd/bake-safety-profile/main.go | 6 +- docs/safety-profiles.md | 8 ++ internal/cmd/kong_helpers.go | 11 +++ internal/cmd/root.go | 7 +- internal/cmd/safety_profile.go | 55 ++++++++++++- internal/cmd/safety_profile_default.go | 4 + .../cmd/safety_profile_locked_flags_test.go | 79 +++++++++++++++++++ 7 files changed, 166 insertions(+), 4 deletions(-) diff --git a/cmd/bake-safety-profile/main.go b/cmd/bake-safety-profile/main.go index b06a6bbd9..294a818d2 100644 --- a/cmd/bake-safety-profile/main.go +++ b/cmd/bake-safety-profile/main.go @@ -71,11 +71,14 @@ func generate(profile *safetyprofile.Profile) []byte { } // writeLockedFlags emits the locked flag lookup. Names are hashed like the command -// rules; the locked values are literals because the flag parser consumes them. +// 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") @@ -90,6 +93,7 @@ func writeLockedFlags(out *bytes.Buffer, flags []safetyprofile.LockedFlag) { 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) { diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 3d7722c19..76b53d685 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -206,6 +206,14 @@ A locked flag behaves as follows: 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: 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 616e446d8..e984a2834 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -181,6 +181,9 @@ 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) } @@ -419,8 +422,8 @@ func applyExplicitOutputModePrecedence(kctx *kong.Context, flags *RootFlags) err jsonLocked := lockedFlagNames["json"] plainLocked := lockedFlagNames["plain"] - jsonSet := flagProvided(kctx, "json") - plainSet := flagProvided(kctx, "plain") + jsonSet := flagOnCommandLine(kctx, "json") + plainSet := flagOnCommandLine(kctx, "plain") switch { case jsonLocked && !plainLocked: if plainSet { diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index bec7f102e..6f2f12af2 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -64,6 +64,59 @@ func lockedFlagsNote() string { 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. @@ -79,7 +132,7 @@ func enforceLockedFlags(kctx *kong.Context) error { if !locked { continue } - if flagProvided(kctx, flag.Name) { + 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 { diff --git a/internal/cmd/safety_profile_default.go b/internal/cmd/safety_profile_default.go index 4be432b62..78eaea1fc 100644 --- a/internal/cmd/safety_profile_default.go +++ b/internal/cmd/safety_profile_default.go @@ -38,3 +38,7 @@ 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 index 8c11e2466..0967a5b43 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -236,3 +236,82 @@ version: true 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) + } +} From 6a7d1de08161eb94dc99513c0b912735de29a957 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 10 Aug 2026 18:41:04 -0700 Subject: [PATCH 10/12] fix(safety-profile): harden locked flag enforcement Co-authored-by: Ronny Rentner --- CHANGELOG.md | 1 + docs/safety-profiles.md | 22 +- internal/cmd/root.go | 17 +- internal/cmd/safety_profile.go | 7 +- .../cmd/safety_profile_locked_flags_test.go | 59 ++- safety-profiles/agent-safe-locked.yaml | 357 ----------------- safety-profiles/readonly-locked.yaml | 363 ------------------ 7 files changed, 80 insertions(+), 746 deletions(-) delete mode 100644 safety-profiles/agent-safe-locked.yaml delete mode 100644 safety-profiles/readonly-locked.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index e0c1c1b6d..fb87de873 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.35.1 - Unreleased +- Safety: allow custom baked profiles to lock CLI flag values against command-line, environment, and config overrides without exposing invalid configured values in errors. (#976) — thanks @ronny-rentner. - Auth: offer one-time re-authorization for expired or revoked stored OAuth refresh tokens after interactive confirmation, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. - Dependencies: update Kong, Cloudflare Workers types, and pnpm to their latest releases. diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 76b53d685..cff020418 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -115,20 +115,6 @@ 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 @@ -193,7 +179,8 @@ locked-flags: ``` Values may be booleans, integers, or strings, and are parsed exactly as the flag -itself parses them. +itself parses them. Locked values are compiled into the binary and are not secret +storage; never use this mechanism for access tokens, passwords, or other credentials. A locked flag behaves as follows: @@ -208,6 +195,7 @@ A locked flag behaves as follows: forbids. - commands that build partial requests from which flags were given treat a locked flag as given, so the locked value reaches the request. +- invalid locked values fail without printing the configured value. 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 @@ -251,6 +239,10 @@ fails because the locked flag demands an argument they had no reason to supply: Flags that only shape output are the safe candidates. +To make an existing preset stricter without changing its behavior for everyone, +copy it and add the locks you need. For example, an agent-oriented copy commonly +locks `sanitize-content`, `wrap-untrusted`, and `no-input` to `true`. + ## Choosing A Profile Use `readonly` when the caller should never change Google or local `gog` state. diff --git a/internal/cmd/root.go b/internal/cmd/root.go index e984a2834..9a65a2edb 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -178,6 +178,13 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { return runtime.Config, nil } + // Treat automatic JSON as an ambient default, like the parser's environment + // and config-backed defaults. Locked flags run afterwards and therefore remain + // authoritative when a profile fixes json to false or plain to true. + if envBool("GOG_AUTO_JSON") && !cli.JSON && !cli.Plain && !isTerminalWriter(runtimeIO.Out) { + cli.JSON = true + } + if err = enforceBakedSafetyProfile(kctx); err != nil { return reportEarlyError(runtimeIO.Err, err) } @@ -212,12 +219,6 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { }))) defer slog.SetDefault(previousLogger) - // Optional automatic JSON output when stdout is piped/non-TTY. - // We intentionally do this after parsing so `--plain` can override it. - if envBool("GOG_AUTO_JSON") && !cli.JSON && !cli.Plain && !isTerminalWriter(runtimeIO.Out) { - cli.JSON = true - } - mode, err := outfmt.FromFlags(cli.JSON, cli.Plain) if err != nil { return reportEarlyError(runtimeIO.Err, newUsageError(err)) @@ -420,8 +421,8 @@ func applyExplicitOutputModePrecedence(kctx *kong.Context, flags *RootFlags) err return nil } - jsonLocked := lockedFlagNames["json"] - plainLocked := lockedFlagNames["plain"] + jsonLocked := lockedFlagNames["json"] && flags.JSON + plainLocked := lockedFlagNames["plain"] && flags.Plain jsonSet := flagOnCommandLine(kctx, "json") plainSet := flagOnCommandLine(kctx, "plain") switch { diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 6f2f12af2..35b45f258 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -135,8 +135,11 @@ func enforceLockedFlags(kctx *kong.Context) error { 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) + if err := flag.Parse(kong.ScanFromTokens(kong.Token{Type: kong.FlagValueToken, Value: value}), flag.Target); err != nil { + // The value is compiled policy data. Do not echo it through parser errors: + // profiles can lock arbitrary strings, and the caller may not be allowed to + // learn the configured value even when the profile is malformed. + return usagef("locked flag --%s has a value that is invalid for that flag", flag.Name) } lockedFlagNames[flag.Name] = true } diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index 0967a5b43..344c20820 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -68,10 +68,11 @@ func TestLockedFlag_IgnoredWithoutBakedProfile(t *testing.T) { // 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) { + const lockedValue = "notabool-private-policy-value" withBakedSafetyProfile(t, ` name: locked locked-flags: - sanitize-content: notabool + sanitize-content: `+lockedValue+` gmail: get: true `) @@ -85,6 +86,9 @@ gmail: if !strings.Contains(result.err.Error(), "locked flag --sanitize-content") { t.Fatalf("err = %v", result.err) } + if strings.Contains(result.err.Error(), lockedValue) || strings.Contains(result.stderr, lockedValue) { + t.Fatalf("locked value leaked through error: err=%v stderr=%q", result.err, result.stderr) + } } // Non-bool locks go through the same flag parser, so prove an int actually lands. @@ -237,6 +241,59 @@ version: true } } +func TestLockedFlag_FalseJSONAllowsPlain(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + json: false +version: true +`) + result := executeWithTestRuntime(t, []string{"--plain", "version"}, nil) + if result.err != nil { + t.Fatalf("false json lock must allow plain: %v (stderr=%q)", result.err, result.stderr) + } + if strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { + t.Fatalf("expected plain output, got %q", result.stdout) + } +} + +func TestLockedFlag_FalsePlainAllowsJSON(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + plain: false +version: true +`) + result := executeWithTestRuntime(t, []string{"--json", "version"}, nil) + if result.err != nil { + t.Fatalf("false plain lock must allow json: %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_FalseJSONOverridesEnvironmentDefaults(t *testing.T) { + for _, key := range []string{"GOG_JSON", "GOG_AUTO_JSON"} { + t.Run(key, func(t *testing.T) { + t.Setenv(key, "1") + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + json: false +version: true +`) + result := executeWithTestRuntime(t, []string{"version"}, nil) + if result.err != nil { + t.Fatalf("false json lock with %s must not fail: %v (stderr=%q)", key, result.err, result.stderr) + } + if strings.HasPrefix(strings.TrimSpace(result.stdout), "{") { + t.Fatalf("%s overrode locked json=false: %q", key, 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) { diff --git a/safety-profiles/agent-safe-locked.yaml b/safety-profiles/agent-safe-locked.yaml deleted file mode 100644 index 8422f1844..000000000 --- a/safety-profiles/agent-safe-locked.yaml +++ /dev/null @@ -1,357 +0,0 @@ -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 deleted file mode 100644 index dc8dd2f96..000000000 --- a/safety-profiles/readonly-locked.yaml +++ /dev/null @@ -1,363 +0,0 @@ -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 From a16e9334583b7d9562a5d896ead8becfe3bfbf1d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 10 Aug 2026 19:06:17 -0700 Subject: [PATCH 11/12] fix(safety-profile): validate locked flag injection Co-authored-by: Ronny Rentner --- docs/safety-profiles.md | 11 +- internal/cmd/root.go | 6 +- internal/cmd/safety_profile.go | 38 +++++- .../cmd/safety_profile_locked_flags_test.go | 121 ++++++++++++++++++ 4 files changed, 165 insertions(+), 11 deletions(-) diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index cff020418..45afe9e91 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -197,11 +197,12 @@ A locked flag behaves as follows: flag as given, so the locked value reaches the request. - invalid locked values fail without printing the configured value. -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. +Some locks are refused rather than half-applied: `--home`, which the layout resolver +reads straight from argv; `--help` and `--version`, which Kong executes before normal +parsing; 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. Other locked values are decoded +from a clean state and pass through the same Kong validation as command-line values. Setting a locked flag fails before the command handler runs: diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 9a65a2edb..691918460 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -141,6 +141,9 @@ func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { if err != nil { return reportEarlyError(runtimeIO.Err, err) } + if err = verifyLockedFlagsExist(parser.Model.Node); err != nil { + return reportEarlyError(runtimeIO.Err, err) + } args = rewriteDocsCellUpdateContentArgs(parser.Model, args) args = rewriteDesirePathArgs(parser.Model, args) @@ -188,9 +191,6 @@ 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) } diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 35b45f258..2d2c95d5a 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "hash/fnv" + "reflect" "sort" "strings" @@ -68,7 +69,7 @@ func lockedFlagsNote() string { // 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 { +func verifyLockedFlagsExist(root *kong.Node) error { want := bakedSafetyLockedFlagCount() if want == 0 { return nil @@ -93,7 +94,7 @@ func verifyLockedFlagsExist(kctx *kong.Context) error { walk(child) } } - walk(kctx.Model.Node) + walk(root) 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)) } @@ -109,6 +110,10 @@ func lockUnsupported(flag *kong.Flag) error { // 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.Name == "help" || flag.Name == "version": + // Kong executes these flags from a BeforeReset hook and exits before normal + // defaults, validation, and locked-flag enforcement run. + return usagef("baked safety profile %q locks --%s, which runs before flags are parsed and cannot be locked", bakedSafetyProfileName(), flag.Name) 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. @@ -127,6 +132,7 @@ func enforceLockedFlags(kctx *kong.Context) error { if !bakedSafetyEnabled() { return nil } + lockedPaths := make([]*kong.Path, 0, bakedSafetyLockedFlagCount()) for _, flag := range kctx.Flags() { value, locked := bakedSafetyLockedFlag(flag.Name) if !locked { @@ -135,13 +141,39 @@ func enforceLockedFlags(kctx *kong.Context) error { if flagOnCommandLine(kctx, flag.Name) { return usagef("flag --%s is locked by baked safety profile %q", flag.Name, bakedSafetyProfileName()) } - if err := flag.Parse(kong.ScanFromTokens(kong.Token{Type: kong.FlagValueToken, Value: value}), flag.Target); err != nil { + // Decode into a zero target first. Kong has already resolved defaults and + // environment values into flag.Target; decoding there would append to slices + // and merge maps instead of replacing them with the locked value. Clear the + // shared Set bit while decoding too: Kong's file/path mappers use it to skip + // defaults after an explicit value and would otherwise skip the lock itself. + lockedTarget := reflect.New(flag.Target.Type()).Elem() + wasSet := flag.Set + flag.Set = false + if err := flag.Parse(kong.ScanFromTokens(kong.Token{Type: kong.FlagValueToken, Value: value}), lockedTarget); err != nil { + flag.Set = wasSet // The value is compiled policy data. Do not echo it through parser errors: // profiles can lock arbitrary strings, and the caller may not be allowed to // learn the configured value even when the profile is malformed. return usagef("locked flag --%s has a value that is invalid for that flag", flag.Name) } + flag.Apply(lockedTarget) lockedFlagNames[flag.Name] = true + lockedPaths = append(lockedPaths, &kong.Path{Flag: flag}) + } + if len(lockedPaths) == 0 { + return nil + } + + // Parsing a mapper is not the whole Kong contract. Revalidate enums, flag and + // command validators, required groups, and xor/and relationships with the locks + // represented as supplied flags. Restore the real command-line trace afterwards + // so override detection continues to mean "typed by the caller". + pathLen := len(kctx.Path) + kctx.Path = append(kctx.Path, lockedPaths...) + validationErr := kctx.Validate() + kctx.Path = kctx.Path[:pathLen] + if validationErr != nil { + return usagef("baked safety profile %q has locked flags that are invalid for the selected command", bakedSafetyProfileName()) } return nil } diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index 344c20820..a928692fa 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -1,8 +1,12 @@ package cmd import ( + "os" + "reflect" "strings" "testing" + + "github.com/alecthomas/kong" ) const lockedFlagsProfile = ` @@ -91,6 +95,97 @@ gmail: } } +func TestLockedFlag_ReplacesResolvedSliceValue(t *testing.T) { + t.Setenv("LOCKED_FLAG_TEST_ITEMS", "ambient") + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + item: profile +version: true +`) + var cli struct { + Items []string `name:"item" env:"LOCKED_FLAG_TEST_ITEMS"` + } + parser, err := kong.New(&cli) + if err != nil { + t.Fatalf("new parser: %v", err) + } + kctx, err := parser.Parse(nil) + if err != nil { + t.Fatalf("parse: %v", err) + } + if err := enforceLockedFlags(kctx); err != nil { + t.Fatalf("enforceLockedFlags: %v", err) + } + if want := []string{"profile"}; !reflect.DeepEqual(cli.Items, want) { + t.Fatalf("locked slice = %#v, want %#v", cli.Items, want) + } +} + +func TestLockedFlag_ReplacesStatefulMapperValue(t *testing.T) { + dir := t.TempDir() + ambient := dir + "/ambient.txt" + locked := dir + "/locked.txt" + if err := os.WriteFile(ambient, []byte("ambient"), 0o600); err != nil { + t.Fatalf("write ambient file: %v", err) + } + if err := os.WriteFile(locked, []byte("locked"), 0o600); err != nil { + t.Fatalf("write locked file: %v", err) + } + t.Setenv("LOCKED_FLAG_TEST_FILE", ambient) + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + file: `+locked+` +version: true +`) + var cli struct { + File string `name:"file" type:"existingfile" env:"LOCKED_FLAG_TEST_FILE"` + } + parser, err := kong.New(&cli) + if err != nil { + t.Fatalf("new parser: %v", err) + } + kctx, err := parser.Parse(nil) + if err != nil { + t.Fatalf("parse: %v", err) + } + if err := enforceLockedFlags(kctx); err != nil { + t.Fatalf("enforceLockedFlags: %v", err) + } + if cli.File != locked { + t.Fatalf("locked existingfile = %q, want %q", cli.File, locked) + } +} + +func TestLockedFlag_RevalidatesInjectedValueWithoutLeakingIt(t *testing.T) { + const lockedValue = "private-invalid-mode" + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + mode: `+lockedValue+` +version: true +`) + var cli struct { + Mode string `name:"mode" enum:"safe,strict" default:"safe"` + } + parser, err := kong.New(&cli) + if err != nil { + t.Fatalf("new parser: %v", err) + } + kctx, err := parser.Parse(nil) + if err != nil { + t.Fatalf("parse: %v", err) + } + err = enforceLockedFlags(kctx) + if err == nil { + t.Fatal("invalid locked enum must fail") + } + if strings.Contains(err.Error(), lockedValue) { + t.Fatalf("locked value leaked through validation error: %v", err) + } +} + // Non-bool locks go through the same flag parser, so prove an int actually lands. func TestLockedFlag_NonBoolValueReachesCommand(t *testing.T) { withBakedSafetyProfile(t, ` @@ -351,6 +446,32 @@ version: true } } +func TestLockedFlag_PreParseFlagsAreRefusedBeforeTheyCanExit(t *testing.T) { + for _, tc := range []struct { + name string + args []string + }{ + {name: "help", args: []string{"--help"}}, + {name: "version", args: []string{"--version"}}, + } { + t.Run(tc.name, func(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + `+tc.name+`: false +version: true +`) + result := executeWithTestRuntime(t, tc.args, nil) + if result.err == nil { + t.Fatalf("locking --%s must fail before Kong exits, got stdout=%q", tc.name, result.stdout) + } + if !strings.Contains(result.err.Error(), "runs before flags are parsed") { + 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) { From 6d30196ff9126f65d6d139a1dece03ce3044c7ba Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 10 Aug 2026 19:30:15 -0700 Subject: [PATCH 12/12] refactor(safety-profile): limit locked flags to booleans Co-authored-by: Ronny Rentner --- CHANGELOG.md | 2 +- cmd/bake-safety-profile/main.go | 2 +- docs/safety-profiles.md | 21 +- internal/cmd/root.go | 8 +- internal/cmd/safety_profile.go | 23 +-- .../cmd/safety_profile_locked_flags_test.go | 186 ++---------------- internal/safetyprofile/locked_flags_test.go | 23 ++- internal/safetyprofile/parse.go | 14 +- internal/safetyprofile/profile.go | 4 +- 9 files changed, 73 insertions(+), 210 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb87de873..811ebf66f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.35.1 - Unreleased -- Safety: allow custom baked profiles to lock CLI flag values against command-line, environment, and config overrides without exposing invalid configured values in errors. (#976) — thanks @ronny-rentner. +- Safety: allow custom baked profiles to lock boolean CLI flags against command-line, environment, and config overrides without echoing the locked value in override errors. (#976) — thanks @ronny-rentner. - Auth: offer one-time re-authorization for expired or revoked stored OAuth refresh tokens after interactive confirmation, while preserving non-interactive recovery guidance. (#973) — thanks @inamiy. - Dependencies: update Kong, Cloudflare Workers types, and pnpm to their latest releases. diff --git a/cmd/bake-safety-profile/main.go b/cmd/bake-safety-profile/main.go index 294a818d2..4b152bead 100644 --- a/cmd/bake-safety-profile/main.go +++ b/cmd/bake-safety-profile/main.go @@ -71,7 +71,7 @@ func generate(profile *safetyprofile.Profile) []byte { } // 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 +// rules; the locked boolean 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) { diff --git a/docs/safety-profiles.md b/docs/safety-profiles.md index 45afe9e91..9e5a07843 100644 --- a/docs/safety-profiles.md +++ b/docs/safety-profiles.md @@ -175,12 +175,11 @@ 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. Locked values are compiled into the binary and are not secret -storage; never use this mechanism for access tokens, passwords, or other credentials. +Locked values and the flags they target must be boolean. This intentionally keeps +the mechanism focused on safety policy rather than compiling account names, paths, +tokens, or other arbitrary configuration into a binary. A locked flag behaves as follows: @@ -195,14 +194,12 @@ A locked flag behaves as follows: forbids. - commands that build partial requests from which flags were given treat a locked flag as given, so the locked value reaches the request. -- invalid locked values fail without printing the configured value. +- override errors name the flag and profile without printing the locked value. -Some locks are refused rather than half-applied: `--home`, which the layout resolver -reads straight from argv; `--help` and `--version`, which Kong executes before normal -parsing; 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. Other locked values are decoded -from a clean state and pass through the same Kong validation as command-line values. +Non-boolean flags are refused. `--help` and `--version`, which Kong executes before +normal parsing, are refused too, as are `--home`, required flags, and names matching +no flag at all. 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: @@ -223,7 +220,7 @@ 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 +Locking a boolean 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: diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 691918460..12e2404bd 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -119,6 +119,7 @@ func Execute(args []string) (err error) { } func executeWithRuntime(args []string, runtime *app.Runtime) (err error) { + resetLockedFlagState() runtime = normalizedRuntime(runtime) runtimeIO := runtime.IO @@ -448,7 +449,7 @@ func reportEarlyError(w io.Writer, err error) error { if err == nil { return nil } - msg := strings.TrimSpace(errfmt.Format(err)) + msg := errorMessage(err) if msg != "" { _, _ = fmt.Fprintln(w, msg) } @@ -461,7 +462,10 @@ func reportEarlyError(w io.Writer, err error) error { // 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 { + if msg == "" { + return msg + } + if ExitCode(err) != 2 { return msg } note := lockedFlagsNote() diff --git a/internal/cmd/safety_profile.go b/internal/cmd/safety_profile.go index 2d2c95d5a..9ef1d405a 100644 --- a/internal/cmd/safety_profile.go +++ b/internal/cmd/safety_profile.go @@ -50,6 +50,10 @@ func enforceBakedSafetyProfile(kctx *kong.Context) error { // value it never received on the command line can say where that value came from. var lockedFlagNames = map[string]bool{} +func resetLockedFlagState() { + 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. @@ -118,6 +122,8 @@ func lockUnsupported(flag *kong.Flag) error { // 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) + case !flag.IsBool(): + return usagef("baked safety profile %q locks --%s, but only boolean flags can be locked", bakedSafetyProfileName(), flag.Name) } return nil } @@ -128,7 +134,7 @@ func lockUnsupported(flag *kong.Flag) error { 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{} + resetLockedFlagState() if !bakedSafetyEnabled() { return nil } @@ -141,20 +147,11 @@ func enforceLockedFlags(kctx *kong.Context) error { if flagOnCommandLine(kctx, flag.Name) { return usagef("flag --%s is locked by baked safety profile %q", flag.Name, bakedSafetyProfileName()) } - // Decode into a zero target first. Kong has already resolved defaults and - // environment values into flag.Target; decoding there would append to slices - // and merge maps instead of replacing them with the locked value. Clear the - // shared Set bit while decoding too: Kong's file/path mappers use it to skip - // defaults after an explicit value and would otherwise skip the lock itself. + // Decode into a zero target so the locked boolean replaces any environment or + // default value already resolved by Kong. lockedTarget := reflect.New(flag.Target.Type()).Elem() - wasSet := flag.Set - flag.Set = false if err := flag.Parse(kong.ScanFromTokens(kong.Token{Type: kong.FlagValueToken, Value: value}), lockedTarget); err != nil { - flag.Set = wasSet - // The value is compiled policy data. Do not echo it through parser errors: - // profiles can lock arbitrary strings, and the caller may not be allowed to - // learn the configured value even when the profile is malformed. - return usagef("locked flag --%s has a value that is invalid for that flag", flag.Name) + return usagef("locked boolean flag --%s could not be applied", flag.Name) } flag.Apply(lockedTarget) lockedFlagNames[flag.Name] = true diff --git a/internal/cmd/safety_profile_locked_flags_test.go b/internal/cmd/safety_profile_locked_flags_test.go index a928692fa..579aed250 100644 --- a/internal/cmd/safety_profile_locked_flags_test.go +++ b/internal/cmd/safety_profile_locked_flags_test.go @@ -1,12 +1,8 @@ package cmd import ( - "os" - "reflect" "strings" "testing" - - "github.com/alecthomas/kong" ) const lockedFlagsProfile = ` @@ -69,144 +65,6 @@ func TestLockedFlag_IgnoredWithoutBakedProfile(t *testing.T) { } } -// 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) { - const lockedValue = "notabool-private-policy-value" - withBakedSafetyProfile(t, ` -name: locked -locked-flags: - sanitize-content: `+lockedValue+` -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) - } - if strings.Contains(result.err.Error(), lockedValue) || strings.Contains(result.stderr, lockedValue) { - t.Fatalf("locked value leaked through error: err=%v stderr=%q", result.err, result.stderr) - } -} - -func TestLockedFlag_ReplacesResolvedSliceValue(t *testing.T) { - t.Setenv("LOCKED_FLAG_TEST_ITEMS", "ambient") - withBakedSafetyProfile(t, ` -name: locked -locked-flags: - item: profile -version: true -`) - var cli struct { - Items []string `name:"item" env:"LOCKED_FLAG_TEST_ITEMS"` - } - parser, err := kong.New(&cli) - if err != nil { - t.Fatalf("new parser: %v", err) - } - kctx, err := parser.Parse(nil) - if err != nil { - t.Fatalf("parse: %v", err) - } - if err := enforceLockedFlags(kctx); err != nil { - t.Fatalf("enforceLockedFlags: %v", err) - } - if want := []string{"profile"}; !reflect.DeepEqual(cli.Items, want) { - t.Fatalf("locked slice = %#v, want %#v", cli.Items, want) - } -} - -func TestLockedFlag_ReplacesStatefulMapperValue(t *testing.T) { - dir := t.TempDir() - ambient := dir + "/ambient.txt" - locked := dir + "/locked.txt" - if err := os.WriteFile(ambient, []byte("ambient"), 0o600); err != nil { - t.Fatalf("write ambient file: %v", err) - } - if err := os.WriteFile(locked, []byte("locked"), 0o600); err != nil { - t.Fatalf("write locked file: %v", err) - } - t.Setenv("LOCKED_FLAG_TEST_FILE", ambient) - withBakedSafetyProfile(t, ` -name: locked -locked-flags: - file: `+locked+` -version: true -`) - var cli struct { - File string `name:"file" type:"existingfile" env:"LOCKED_FLAG_TEST_FILE"` - } - parser, err := kong.New(&cli) - if err != nil { - t.Fatalf("new parser: %v", err) - } - kctx, err := parser.Parse(nil) - if err != nil { - t.Fatalf("parse: %v", err) - } - if err := enforceLockedFlags(kctx); err != nil { - t.Fatalf("enforceLockedFlags: %v", err) - } - if cli.File != locked { - t.Fatalf("locked existingfile = %q, want %q", cli.File, locked) - } -} - -func TestLockedFlag_RevalidatesInjectedValueWithoutLeakingIt(t *testing.T) { - const lockedValue = "private-invalid-mode" - withBakedSafetyProfile(t, ` -name: locked -locked-flags: - mode: `+lockedValue+` -version: true -`) - var cli struct { - Mode string `name:"mode" enum:"safe,strict" default:"safe"` - } - parser, err := kong.New(&cli) - if err != nil { - t.Fatalf("new parser: %v", err) - } - kctx, err := parser.Parse(nil) - if err != nil { - t.Fatalf("parse: %v", err) - } - err = enforceLockedFlags(kctx) - if err == nil { - t.Fatal("invalid locked enum must fail") - } - if strings.Contains(err.Error(), lockedValue) { - t.Fatalf("locked value leaked through validation error: %v", 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{ @@ -434,7 +292,7 @@ func TestLockedFlag_HomeIsRefused(t *testing.T) { withBakedSafetyProfile(t, ` name: locked locked-flags: - home: /tmp/locked-home + home: false version: true `) result := executeWithTestRuntime(t, []string{"version"}, nil) @@ -446,6 +304,26 @@ version: true } } +func TestLockedFlag_NonBooleanFlagIsRefused(t *testing.T) { + withBakedSafetyProfile(t, ` +name: locked +locked-flags: + format: true +gmail: + get: true +`) + result := executeWithTestRuntime(t, []string{ + "--account", "nobody@example.invalid", + "gmail", "get", "m1", + }, nil) + if result.err == nil { + t.Fatal("locking non-boolean --format must fail") + } + if !strings.Contains(result.err.Error(), "only boolean flags can be locked") { + t.Fatalf("err = %v", result.err) + } +} + func TestLockedFlag_PreParseFlagsAreRefusedBeforeTheyCanExit(t *testing.T) { for _, tc := range []struct { name string @@ -471,25 +349,3 @@ version: true }) } } - -// 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/safetyprofile/locked_flags_test.go b/internal/safetyprofile/locked_flags_test.go index cd494de3d..9d4206272 100644 --- a/internal/safetyprofile/locked_flags_test.go +++ b/internal/safetyprofile/locked_flags_test.go @@ -7,8 +7,7 @@ func TestParse_LockedFlagsValues(t *testing.T) { name: locked locked-flags: sanitize-content: true - inline-max-bytes: 8388608 - format: metadata + wrap-untrusted: false gmail: get: true `) @@ -16,9 +15,8 @@ gmail: t.Fatalf("Parse: %v", err) } want := []LockedFlag{ - {Name: "format", Value: "metadata"}, - {Name: "inline-max-bytes", Value: "8388608"}, {Name: "sanitize-content", Value: "true"}, + {Name: "wrap-untrusted", Value: "false"}, } if len(profile.LockedFlags) != len(want) { t.Fatalf("locked flags = %#v", profile.LockedFlags) @@ -30,6 +28,23 @@ gmail: } } +func TestParse_LockedFlagsRejectsNonBooleanValues(t *testing.T) { + for _, value := range []string{"1", "metadata"} { + t.Run(value, func(t *testing.T) { + _, err := Parse(` +name: locked +locked-flags: + format: ` + value + ` +gmail: + get: true +`) + if err == nil { + t.Fatal("non-boolean locked value must be rejected") + } + }) + } +} + func TestParse_LockedFlagsRejectsNonScalar(t *testing.T) { _, err := Parse(` name: locked diff --git a/internal/safetyprofile/parse.go b/internal/safetyprofile/parse.go index 6c83dc7e1..6b2c5f905 100644 --- a/internal/safetyprofile/parse.go +++ b/internal/safetyprofile/parse.go @@ -3,7 +3,6 @@ package safetyprofile import ( "fmt" "sort" - "strconv" "strings" "go.yaml.in/yaml/v3" @@ -141,16 +140,11 @@ func addLockedFlags(out map[string]string, value any) error { 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) + value, ok := raw.(bool) + if !ok { + return fmt.Errorf("%s: expected boolean value, got %T", flag, raw) } + out[flag] = fmt.Sprintf("%t", value) } return nil } diff --git a/internal/safetyprofile/profile.go b/internal/safetyprofile/profile.go index f1acc7f05..f314c912c 100644 --- a/internal/safetyprofile/profile.go +++ b/internal/safetyprofile/profile.go @@ -19,8 +19,8 @@ type Profile struct { LockedFlags []LockedFlag } -// LockedFlag is one locked flag. Value is the literal the flag parses, in the same -// form an environment variable would supply. +// LockedFlag is one locked boolean flag. Value is "true" or "false" in the form +// the flag parser consumes. type LockedFlag struct { Name string Value string