Skip to content

feat: compile-time safety profiles for command removal#1

Merged
drewburchfield merged 3 commits intomainfrom
feat/safety-profiles
Feb 25, 2026
Merged

feat: compile-time safety profiles for command removal#1
drewburchfield merged 3 commits intomainfrom
feat/safety-profiles

Conversation

@drewburchfield
Copy link
Owner

@drewburchfield drewburchfield commented Feb 25, 2026

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

  1. feat: Full safety profiles system (YAML config, code generator, build tags, presets)
  2. refactor: AST auto-discovery replacing 600-line manual registry
  3. fix: Quality gate findings (buildEmptyStruct bug, fail-closed tests, open alias fix)

Key files to review

  • cmd/gen-safety/main.go - Code generator pipeline
  • cmd/gen-safety/discover.go - AST auto-discovery engine
  • cmd/gen-safety/discover_test.go + main_test.go - 15 tests
  • build-safe.sh - Build script
  • safety-profile.example.yaml - Example config with risk annotations

Test plan

  • Stock go build unchanged
  • All 15 tests pass
  • All 3 preset profiles build with --strict
  • keep: false compiles correctly (was a bug, now fixed)
  • Fail-closed contract verified by unit tests

Open with Devin

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
@drewburchfield drewburchfield self-assigned this Feb 25, 2026
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

Comment on lines +231 to +237
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)
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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:

  1. generateServiceFile for spec yamlKey="gmail.settings"
  2. isServiceDisabled(profile, "gmail.settings")false (not disabled)
  3. svcConfig = resolveDottedSection(profile, "gmail.settings")nil (gmail is bool, not map)
  4. resolveEnabledFields(fields, nil, profile, "gmail.settings")isServiceEnabledBool returns falsefilterFields(fields, nil) → empty list
  5. Generated GmailSettingsCmd struct 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@drewburchfield drewburchfield merged commit 8f8007b into main Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant