feat: compile-time safety profiles for command removal#1
feat: compile-time safety profiles for command removal#1drewburchfield merged 3 commits intomainfrom
Conversation
Add a build-time code generation system that produces restricted CLI binaries from a YAML configuration. Disabled commands are removed at compile time via Go build tags, so they cannot be invoked at all. How it works: - Parent struct definitions extracted to *_types.go (build tag !safety_profile) - cmd/gen-safety reads a YAML profile and generates *_cmd_gen.go files with build tag safety_profile, containing only the enabled commands - Building with -tags safety_profile uses the generated structs - Stock "go build" is completely unchanged Key design decisions: - Fail-closed: commands not listed in YAML are excluded by default - Each service section can be toggled with enabled: true/false - Individual subcommands can be selectively included or excluded - Utility commands (version, auth, config, completion) always included Includes: - cmd/gen-safety: code generator with YAML validation and --strict mode - cmd/extract-types: one-time tool for extracting parent structs - build-safe.sh: convenience script (generate + compile) - Preset profiles: full.yaml, readonly.yaml, agent-safe.yaml - Example profile: safety-profile.yaml Also fixes contacts_crud.go parameter type grouping (given/org were bool instead of string), which is an existing upstream bug.
Replace the ~655-line hand-maintained command registry in gen-safety with automatic discovery via Go's AST package. The generator now parses *_types.go source files directly to find all Cmd structs and their hierarchy, eliminating manual sync when upstream adds commands. What changed: - New discover.go: parses *_types.go files, walks from CLI struct down to build serviceSpec list and CLI field categorization automatically - New discover_test.go: 10 tests covering tag parsing, file parsing, multi-struct files, NonCmdPrefix, field categorization, and more - main.go: wired to call AST discovery instead of manual registry; ~655 lines of spec functions deleted - Rename safety-profile.yaml to safety-profile.example.yaml (users should copy and customize, not edit the example directly) - Updated Makefile, build-safe.sh, README.md for the rename The generated output is identical (verified by diffing all *_cmd_gen.go files before and after). Net change: ~400 fewer lines of code and no more manual updates when upstream adds commands.
- buildEmptyStruct: preserve non-command fields (e.g. KeepCmd's ServiceAccount/Impersonate) when service is fully disabled - mapHasEnabledLeaf: fatal on unexpected YAML types instead of silently ignoring (matches isEnabled behavior) - isServiceDisabled: warn on unexpected types before fail-closed - Remove misleading `open` key from utility section (it's an alias, not a utility, and is controllable via aliases.open) - Add main_test.go with tests for fail-closed security contract: isEnabled, filterFields, isServiceDisabled, resolveEnabledFields, mapHasEnabledLeaf (15 total tests now) - Fix doc comments, remove dead code, fix build-safe.sh version suffix
| func resolveEnabledFields(fields []field, svcConfig map[string]any, profile map[string]any, yamlKey string) []field { | ||
| if svcConfig == nil && isServiceEnabledBool(profile, yamlKey) { | ||
| // Bool shorthand: `service: true` means include all fields. | ||
| return fields | ||
| } | ||
| return filterFields(fields, svcConfig) | ||
| } |
There was a problem hiding this comment.
🔴 Bool shorthand service: true fails to propagate to nested parent command specs
When a top-level service key uses the true bool shorthand (e.g., gmail: true), nested parent command structs (like GmailSettingsCmd, GmailThreadCmd, etc.) are generated with zero subcommand fields instead of all fields being enabled.
Root Cause and Impact
The resolveEnabledFields function at cmd/gen-safety/main.go:231-237 checks isServiceEnabledBool(profile, yamlKey) when svcConfig is nil. For a dotted key like "gmail.settings", isServiceEnabledBool at cmd/gen-safety/main.go:240-261 walks the YAML tree and requires the last key segment to resolve to a bool. When the profile has gmail: true (top-level bool), the walk resolves gmail to true at index 0, but since i != len(parts)-1, it falls through to the map type assertion, which fails (a bool is not a map[string]any), and returns false.
Meanwhile, isServiceDisabled(profile, "gmail.settings") at cmd/gen-safety/main.go:384-406 resolves gmail to true and returns !true = false (not disabled). So the service is treated as NOT disabled but also NOT enabled-by-bool-shorthand. This causes filterFields(fields, nil) to be called, which excludes ALL subcommand fields (fail-closed).
Trace for gmail: true with nested gmail.settings:
generateServiceFilefor specyamlKey="gmail.settings"isServiceDisabled(profile, "gmail.settings")→false(not disabled)svcConfig = resolveDottedSection(profile, "gmail.settings")→nil(gmail is bool, not map)resolveEnabledFields(fields, nil, profile, "gmail.settings")→isServiceEnabledBoolreturnsfalse→filterFields(fields, nil)→ empty list- Generated
GmailSettingsCmdstruct has zero command fields
The parent GmailCmd correctly includes ALL fields (including Settings GmailSettingsCmd), so the CLI shows a gmail settings command that has no subcommands — the opposite of what gmail: true intends.
Impact: Any user writing a custom profile with gmail: true (or any service with nested parent commands set to true) would get a binary where nested command groups are empty. The README documents this shorthand: "Setting a service key to false disables all of its subcommands" — implying true enables them all. None of the provided preset profiles trigger this bug because they all use explicit per-command maps for services with nested parents.
Prompt for agents
In cmd/gen-safety/main.go, the resolveEnabledFields function (lines 231-237) needs to handle the case where a parent key in the dotted YAML path is set to true (bool shorthand). Currently isServiceEnabledBool only returns true when the LAST segment of the dotted key resolves to a bool. It should also return true when ANY ancestor segment resolves to true.
Fix isServiceEnabledBool (lines 240-261) to check if any prefix of the dotted key resolves to true. Specifically, change the condition on line 251 from:
if b, ok := v.(bool); ok && i == len(parts)-1 {
return b
}
to:
if b, ok := v.(bool); ok {
return b
}
This way, if gmail: true is encountered while resolving gmail.settings, the function returns true immediately (all descendants are enabled). If gmail: false is encountered, it returns false. This matches the behavior of isServiceDisabled which already stops at the first bool in the path.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Adds compile-time safety profiles that physically remove CLI commands from the binary. Disabled commands don't exist in the binary, don't appear in
--help, and can't be invoked.This is a staging PR on my fork for Devin review before pushing to upstream (steipete#366).
Commits
Key files to review
cmd/gen-safety/main.go- Code generator pipelinecmd/gen-safety/discover.go- AST auto-discovery enginecmd/gen-safety/discover_test.go+main_test.go- 15 testsbuild-safe.sh- Build scriptsafety-profile.example.yaml- Example config with risk annotationsTest plan
go buildunchanged--strictkeep: falsecompiles correctly (was a bug, now fixed)