Add safe dry-run previews for mutations by leet-c1 · Pull Request #154 · ConductorOne/cone

leet-c1 · 2026-09-19T02:14:17Z

Summary

add root-level --dry-run previews for supported task mutations, alias generation, and MCP setup
validate and render effective access-request intent before previewing without sending mutations
reject --dry-run on unsupported commands while preserving legacy preview behavior through the unified flag

Validation

go test ./...
go test -race ./...
go vet ./...
live-tested global previews, unsupported-command rejection, and input validation against leet.conductor.one
adversarial security review completed

Static analysis

golangci-lint run ./... and gosec ./... retain five findings in pre-existing AWS code outside this PR (cmd/cone/aws.go).

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>

github-actions · 2026-09-19T02:19:10Z

+}
+
+func dryRunEnabled(cmd *cobra.Command) bool {
+	enabled, _ := cmd.Flags().GetBool(dryRunFlag)


🟡 Suggestion: dryRunEnabled reads the pflag directly, but dry-run is also viper-bound (initConfig → BindPFlags, and getSubViperForProfile with AutomaticEnv). Since generate_alias.go:180 reads it as v.GetBool("dry-run"), CONE_DRY_RUN=true (or dry-run: true in config) previews for generate-alias but is silently ignored for get/drop/task approve|deny|comment|escalate — the real mutation is sent. Consider resolving dry-run through viper everywhere so the env/config path can't silently fail open on a safety flag.

github-actions · 2026-09-19T02:19:16Z

+		if dryRunEnabled(cmd) {
+			previewMutations(cmd, c, v, mutation{
+				Action:  "Create access request",
+				Target:  fmt.Sprintf("app %s, entitlement %s", appID, entitlementID),
+				Details: input.previewDetails(userID),
+			})
+			return nil, nil
 		}


🟡 Suggestion: previewMutations is documented as returning "true when the caller must return without making a change", and the other five call sites use if previewMutations(...) { return nil }. Here (and in runDrop at line 343) the return value is discarded and the gate is duplicated as a separate dryRunEnabled(cmd) check, so any future change to previewMutations' gating would silently diverge in exactly the two commands that create tasks. if previewMutations(...) { return nil, nil } keeps a single source of truth.

github-actions · 2026-09-19T02:19:19Z

+			t.Fatalf("%s unexpectedly supports --dry-run", cmd.CommandPath())
+		}
+	}
+}


🟡 Suggestion: the new tests cover the printer and the annotation scoping, but nothing asserts the actual safety property — that --dry-run prevents CreateGrantTask/CreateRevokeTask/ApproveTask/DenyTask/CommentOnTask/EscalateTask from being called. A fake client.C1Client that fails the test if any mutating method fires would lock that in and catch a future refactor that reorders the preview check past the API call.

github-actions · 2026-09-19T02:19:24Z

 	cliCmd.PersistentFlags().StringP("output", "o", "table", "Output format. Valid values: table, json, json-pretty, wide.")
 	cliCmd.PersistentFlags().Bool("debug", false, "Enable HTTP debug logging")
 	cliCmd.PersistentFlags().String("log-level", "", "Set log level (debug, info, warn, error)")
+	cliCmd.PersistentFlags().Bool(dryRunFlag, false, "Preview supported mutations without sending them")


🟡 Suggestion: as a root persistent flag, --dry-run now shows up under "Global Flags" in every command's help (cone login --help, cone secret create --help, …) while PersistentPreRunE rejects it at runtime for all but eight commands. Consider cmd.PersistentFlags().MarkHidden plus per-command re-exposure, or at least mentioning the supported command list in the flag usage string, so the help output matches what actually works.

github-actions · 2026-09-19T02:26:05Z

General PR Review: Add safe dry-run previews for mutations

Blocking Issues: 0 | Suggestions: 4 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base 5ffe74330f1c.
Review mode: full
View review run: https://github.com/ConductorOne/cone/actions/runs/35415063973

Review Summary

Scanned the full PR diff for security and correctness: the new cmd/cone/mutation.go preview helper, the root --dry-run persistent flag plus its PersistentPreRunE allowlist gate, the runGet/buildGetTaskInput refactor, the five previewMutations call sites, and the initConfig flag-binding fix. I verified that every command carrying the supportsDryRun annotation returns before its mutating client call, that the new task == nil sentinel in runTask cannot skip output on a real request, and that moving dry-run off the generate-alias/install-mcp local flags onto a root persistent flag keeps both the viper read and the pflag read resolving correctly through the cobra inherited-flag merge. No blocking security or correctness issues found; the four suggestions concern dry-run resolution consistency, test coverage of the safety property, and help-output accuracy.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

cmd/cone/mutation.go:40 — dryRunEnabled reads the pflag directly while generate_alias.go:180 reads the same flag through viper, so CONE_DRY_RUN=true or a config-file dry-run: true previews for generate-alias but is silently ignored for get/drop/task approve|deny|comment|escalate, sending the real mutation.
cmd/cone/mutation_test.go:73 — no test asserts the actual safety property, that --dry-run prevents the mutating C1Client calls from firing.
cmd/cone/get_drop_task.go:291 — runGet and runDrop discard the documented return value of previewMutations and duplicate the gate as a separate dryRunEnabled(cmd) check, diverging from the other five call sites.
cmd/cone/main.go:64 — --dry-run appears under Global Flags in every command help output but is rejected at runtime for all but eight commands.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `cmd/cone/mutation.go`:
- Around line 39-41: `dryRunEnabled` resolves the dry-run flag with
  `cmd.Flags().GetBool(dryRunFlag)`, reading only the pflag value. But `dry-run` is
  also bound into viper: `initConfig` calls `viper.BindPFlags` on the root persistent
  flags, and `getSubViperForProfile` binds `cmd.Flags()` with `AutomaticEnv` and a
  `-` to `_` key replacer. `cmd/cone/generate_alias.go:180` reads the same flag as
  `v.GetBool("dry-run")` instead. The result is split behavior: with
  `CONE_DRY_RUN=true` in the environment, or `dry-run: true` in
  `~/.conductorone/config.yaml`, `cone generate-alias` previews, while
  `cone drop <alias>` and `cone task approve|deny|comment|escalate` ignore the
  setting and send the real mutation. Fix by resolving dry-run through viper in one
  place: change `dryRunEnabled` to take the `*viper.Viper` returned by `cmdContext`
  and return `v.GetBool(dryRunFlag)`, have `previewMutations` use that single
  resolver, and make `generate_alias.go` and `install_mcp.go` use it too instead of
  their own reads. Note that the root `PersistentPreRunE` gate in `cmd/cone/main.go`
  also calls `dryRunEnabled` before any per-command viper instance exists; if
  threading viper there is awkward, read the flag with a fallback that also consults
  the global viper, so the env and config path cannot silently fail open on a safety
  flag.

In `cmd/cone/mutation_test.go`:
- Around line 52-72: the new tests cover the preview printer and which commands carry
  the dry-run annotation, but nothing verifies the property the feature exists for,
  that `--dry-run` stops the mutating API call. Add a test with a fake
  `client.C1Client` whose `CreateGrantTask`, `CreateRevokeTask`, `ApproveTask`,
  `DenyTask`, `CommentOnTask` and `EscalateTask` implementations call `t.Fatal`, then
  drive each dry-run command path against it and assert the command returns nil with
  preview output written to the out stream of the command. This locks in the ordering
  so a future refactor cannot move a preview check to after the API call.

In `cmd/cone/get_drop_task.go`:
- Around line 291-298 and 343-353: `previewMutations` is documented as returning true
  when the caller must return without making a change, and the five call sites in
  `task_approve_deny.go`, `task_comment.go` and `task_escalate.go` branch on that
  return value directly, as `if previewMutations(...) { return nil }`. In
  `runGet` and `runDrop` the return value is discarded and the gate is instead
  duplicated as a separate `if dryRunEnabled(cmd)` check wrapping the call. Collapse
  both to `if previewMutations(cmd, c, v, mutation{...}) { return nil, nil }`
  so there is a single source of truth for the gating condition; otherwise a future
  change to the dry-run check inside `previewMutations` would silently diverge in
  exactly the two commands that create tasks.

In `cmd/cone/main.go`:
- Around line 64: registering `dry-run` as a root persistent flag makes it appear in
  the Global Flags section of help for every command, for example `cone login
  --help`, `cone secret create --help` and `cone whoami --help`, even though the
  `PersistentPreRunE` check at lines 46-48 rejects it at runtime for every command
  without the `supportsDryRun` annotation. Either mark the persistent flag hidden and
  re-expose it per supporting command, or extend the usage string to name the
  supported commands, so help output matches the commands that actually accept it.

github-actions

No blocking issues found.

feat: add dry-run previews for mutations

e53118c

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>

github-actions Bot reviewed Sep 19, 2026

View reviewed changes

Navigation Menu

Add safe dry-run previews for mutations - #154

Add safe dry-run previews for mutations#154
leet-c1 wants to merge 1 commit into
mainfrom
lee.tschetter/cone-dry-run-previews

leet-c1 commented Sep 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot Sep 19, 2026

Uh oh!

github-actions Bot Sep 19, 2026

Uh oh!

github-actions Bot Sep 19, 2026

Uh oh!

github-actions Bot Sep 19, 2026

Uh oh!

github-actions Bot commented Sep 19, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

leet-c1 commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Static analysis

Uh oh!

github-actions Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 19, 2026

General PR Review: Add safe dry-run previews for mutations

Review Summary

Security Issues

Correctness Issues

Suggestions

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

leet-c1 commented Sep 19, 2026 •

edited

Loading