-
Notifications
You must be signed in to change notification settings - Fork 589
Feat/risk tiering #633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Feat/risk tiering #633
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ebb74ba
feat(risk): implement confirmation for high-risk write operations
sang-neo03 4c61a55
feat(risk): streamline confirmation for high-risk write operations
sang-neo03 1b7b9d6
Merge branch 'main' into feat/risk-tiering
sang-neo03 f54083c
feat(risk): document approval protocol for high-risk write operations
sang-neo03 6af1a2a
feat(risk): refine confirmation protocol for high-risk write operations
sang-neo03 709cabe
feat(fix): fix conflict
sang-neo03 0bf426d
feat(risk): remove redundant variable declaration in risk test
sang-neo03 5248b08
feat(risk): add 'Yes' flag to various test cases for confirmation
sang-neo03 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/larksuite/cli/internal/cmdutil" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // rendersHelp runs the wrapped help func and returns stdout. | ||
| func rendersHelp(t *testing.T, cmd *cobra.Command) string { | ||
| t.Helper() | ||
| var buf bytes.Buffer | ||
| cmd.SetOut(&buf) | ||
| cmd.SetErr(&buf) | ||
| cmd.HelpFunc()(cmd, nil) | ||
| return buf.String() | ||
| } | ||
|
|
||
| func TestHelpFunc_RendersRiskLineWhenAnnotated(t *testing.T) { | ||
| root := &cobra.Command{Use: "lark-cli"} | ||
| installTipsHelpFunc(root) | ||
|
|
||
| child := &cobra.Command{Use: "delete", Short: "delete a file"} | ||
| cmdutil.SetRisk(child, "high-risk-write") | ||
| root.AddCommand(child) | ||
|
|
||
| out := rendersHelp(t, child) | ||
| if !strings.Contains(out, "Risk: high-risk-write") { | ||
| t.Errorf("expected Risk line in help output, got:\n%s", out) | ||
| } | ||
| } | ||
|
|
||
| func TestHelpFunc_NoRiskLineWhenUnannotated(t *testing.T) { | ||
| root := &cobra.Command{Use: "lark-cli"} | ||
| installTipsHelpFunc(root) | ||
|
|
||
| child := &cobra.Command{Use: "list", Short: "list items"} | ||
| root.AddCommand(child) | ||
|
|
||
| out := rendersHelp(t, child) | ||
| if strings.Contains(out, "Risk:") { | ||
| t.Errorf("expected no Risk line when annotation is absent, got:\n%s", out) | ||
| } | ||
| } | ||
|
|
||
| func TestHelpFunc_RiskLinePrecedesTips(t *testing.T) { | ||
| root := &cobra.Command{Use: "lark-cli"} | ||
| installTipsHelpFunc(root) | ||
|
|
||
| child := &cobra.Command{Use: "delete", Short: "delete a file"} | ||
| cmdutil.SetRisk(child, "high-risk-write") | ||
| cmdutil.SetTips(child, []string{"use --yes to confirm"}) | ||
| root.AddCommand(child) | ||
|
|
||
| out := rendersHelp(t, child) | ||
| riskIdx := strings.Index(out, "Risk:") | ||
| tipsIdx := strings.Index(out, "Tips:") | ||
| if riskIdx == -1 || tipsIdx == -1 { | ||
| t.Fatalf("expected both Risk and Tips sections, got:\n%s", out) | ||
| } | ||
| if riskIdx >= tipsIdx { | ||
| t.Errorf("expected Risk to precede Tips; got Risk@%d, Tips@%d", riskIdx, tipsIdx) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package service | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/larksuite/cli/internal/cmdutil" | ||
| ) | ||
|
|
||
| // highRiskDeleteMethod mirrors a simple DELETE API with a required path | ||
| // parameter and risk metadata. The returned map is what service registration | ||
| // reads; the test exercises --yes registration and the gate behavior. | ||
| func highRiskDeleteMethod() map[string]interface{} { | ||
| return map[string]interface{}{ | ||
| "path": "files/{file_token}", | ||
| "httpMethod": "DELETE", | ||
| "risk": "high-risk-write", | ||
| "parameters": map[string]interface{}{ | ||
| "file_token": map[string]interface{}{ | ||
| "type": "string", "location": "path", "required": true, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func writeMethodNoRisk() map[string]interface{} { | ||
| return map[string]interface{}{ | ||
| "path": "files/{file_token}", | ||
| "httpMethod": "DELETE", | ||
| "parameters": map[string]interface{}{ | ||
| "file_token": map[string]interface{}{ | ||
| "type": "string", "location": "path", "required": true, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_YesFlagRegisteredForHighRisk(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), highRiskDeleteMethod(), "delete", "files", nil) | ||
|
|
||
| if cmd.Flags().Lookup("yes") == nil { | ||
| t.Error("expected --yes flag registered for risk=high-risk-write") | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_YesFlagNotRegisteredForWrite(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), writeMethodNoRisk(), "delete", "files", nil) | ||
|
|
||
| if cmd.Flags().Lookup("yes") != nil { | ||
| t.Error("expected --yes flag NOT registered when risk is unset") | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_RiskAnnotationSet(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), highRiskDeleteMethod(), "delete", "files", nil) | ||
|
|
||
| level, ok := cmdutil.GetRisk(cmd) | ||
| if !ok { | ||
| t.Fatal("expected Risk annotation to be set") | ||
| } | ||
| if level != "high-risk-write" { | ||
| t.Errorf("level = %q, want high-risk-write", level) | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_RiskAnnotationAbsentForUnsetRisk(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), writeMethodNoRisk(), "delete", "files", nil) | ||
|
|
||
| if _, ok := cmdutil.GetRisk(cmd); ok { | ||
| t.Error("expected no Risk annotation when meta risk is unset") | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_GateBlocksWithoutYes(t *testing.T) { | ||
| f, _, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), highRiskDeleteMethod(), "delete", "files", nil) | ||
| // --as bot skips the scope check so we reach the gate without external creds. | ||
| cmd.SetArgs([]string{"--as", "bot", "--params", `{"file_token":"tok_abc"}`}) | ||
|
|
||
| err := cmd.Execute() | ||
| if err == nil { | ||
| t.Fatal("expected confirmation error, got nil") | ||
| } | ||
| if !strings.Contains(err.Error(), "requires confirmation") { | ||
| t.Errorf("expected 'requires confirmation' in error, got: %v", err) | ||
| } | ||
| if !strings.Contains(err.Error(), "drive.files.delete") { | ||
| t.Errorf("expected schema path in error action, got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestServiceMethod_DryRunBypassesGate(t *testing.T) { | ||
| f, stdout, _, _ := cmdutil.TestFactory(t, testConfig) | ||
| cmd := NewCmdServiceMethod(f, driveSpec(), highRiskDeleteMethod(), "delete", "files", nil) | ||
| cmd.SetArgs([]string{ | ||
| "--as", "bot", | ||
| "--params", `{"file_token":"tok_abc"}`, | ||
| "--dry-run", | ||
| }) | ||
|
|
||
| if err := cmd.Execute(); err != nil { | ||
| t.Fatalf("dry-run should not hit confirmation gate; got: %v", err) | ||
| } | ||
| if !strings.Contains(stdout.String(), "files/tok_abc") { | ||
| t.Errorf("expected dry-run output to contain URL, got:\n%s", stdout.String()) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmdutil | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/larksuite/cli/internal/output" | ||
| ) | ||
|
|
||
| // RequireConfirmation constructs a confirmation_required error with exit code | ||
| // ExitConfirmationRequired and a structured Risk envelope. Used by both | ||
| // shortcut and service command execution paths when a statically | ||
| // high-risk-write operation has not been confirmed with --yes. | ||
| // | ||
| // action identifies the operation for the agent (e.g. "mail +send", | ||
| // "drive.files.delete"). The envelope does not carry a pre-built retry | ||
| // command: agents already know their original invocation and only need to | ||
| // append --yes per the hint, which keeps the protocol free of shell-quoting | ||
| // pitfalls. | ||
| func RequireConfirmation(action string) error { | ||
| return &output.ExitError{ | ||
| Code: output.ExitConfirmationRequired, | ||
| Detail: &output.ErrDetail{ | ||
| Type: "confirmation_required", | ||
| Message: fmt.Sprintf("%s requires confirmation", action), | ||
| Hint: "add --yes to confirm", | ||
| Risk: &output.RiskDetail{ | ||
| Level: "high-risk-write", | ||
| Action: action, | ||
| }, | ||
| }, | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmdutil | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "errors" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/larksuite/cli/internal/output" | ||
| ) | ||
|
|
||
| func TestRequireConfirmation_EnvelopeShape(t *testing.T) { | ||
| err := RequireConfirmation("drive +delete") | ||
| if err == nil { | ||
| t.Fatal("expected non-nil error") | ||
| } | ||
|
|
||
| var exitErr *output.ExitError | ||
| if !errors.As(err, &exitErr) { | ||
| t.Fatalf("expected *output.ExitError, got %T", err) | ||
| } | ||
| if exitErr.Code != output.ExitConfirmationRequired { | ||
| t.Errorf("Code = %d, want %d", exitErr.Code, output.ExitConfirmationRequired) | ||
| } | ||
| if exitErr.Detail == nil { | ||
| t.Fatal("Detail is nil") | ||
| } | ||
| d := exitErr.Detail | ||
| if d.Type != "confirmation_required" { | ||
| t.Errorf("Type = %q, want confirmation_required", d.Type) | ||
| } | ||
| if !strings.Contains(d.Message, "drive +delete") || !strings.Contains(d.Message, "requires confirmation") { | ||
| t.Errorf("Message = %q, want it to mention action and 'requires confirmation'", d.Message) | ||
| } | ||
| if d.Hint != "add --yes to confirm" { | ||
| t.Errorf("Hint = %q, want 'add --yes to confirm'", d.Hint) | ||
| } | ||
| if d.Risk == nil { | ||
| t.Fatal("Risk is nil") | ||
| } | ||
| if d.Risk.Level != "high-risk-write" { | ||
| t.Errorf("Risk.Level = %q, want high-risk-write", d.Risk.Level) | ||
| } | ||
| if d.Risk.Action != "drive +delete" { | ||
| t.Errorf("Risk.Action = %q, want drive +delete", d.Risk.Action) | ||
| } | ||
| } | ||
|
sang-neo03 marked this conversation as resolved.
|
||
|
|
||
| func TestRequireConfirmation_JSONShape(t *testing.T) { | ||
| err := RequireConfirmation("mail +send") | ||
| var exitErr *output.ExitError | ||
| if !errors.As(err, &exitErr) { | ||
| t.Fatalf("expected *output.ExitError, got %T", err) | ||
| } | ||
| raw, mErr := json.Marshal(exitErr.Detail) | ||
| if mErr != nil { | ||
| t.Fatalf("marshal: %v", mErr) | ||
| } | ||
| var back map[string]interface{} | ||
| if err := json.Unmarshal(raw, &back); err != nil { | ||
| t.Fatalf("unmarshal: %v", err) | ||
| } | ||
|
|
||
| // No fix_command field leaks into the envelope: the protocol avoids | ||
| // shell-quoting hazards by delegating retry to agent-side logic. | ||
| if _, has := back["fix_command"]; has { | ||
| t.Errorf("unexpected fix_command present in JSON: %s", raw) | ||
| } | ||
|
|
||
| risk, ok := back["risk"].(map[string]interface{}) | ||
| if !ok { | ||
| t.Fatalf("risk block missing in JSON: %s", raw) | ||
| } | ||
| if risk["level"] != "high-risk-write" { | ||
| t.Errorf("risk.level in JSON = %v", risk["level"]) | ||
| } | ||
| if risk["action"] != "mail +send" { | ||
| t.Errorf("risk.action in JSON = %v", risk["action"]) | ||
| } | ||
| // Action-only protocol: no UpgradedBy / fix_command / upgraded_by leak. | ||
| if _, has := risk["upgraded_by"]; has { | ||
| t.Errorf("unexpected upgraded_by present in JSON: %s", raw) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmdutil | ||
|
|
||
| import "github.com/spf13/cobra" | ||
|
|
||
| const riskLevelAnnotationKey = "risk_level" | ||
|
|
||
| // SetRisk stores a command's static risk level on cobra annotations so the | ||
| // help renderer (cmd/root.go) can surface a Risk: line without importing | ||
| // shortcuts/common. Levels follow the three-tier convention: "read" | "write" | ||
| // | "high-risk-write". Framework-level confirmation gating only acts on | ||
| // "high-risk-write". | ||
| func SetRisk(cmd *cobra.Command, level string) { | ||
| if level == "" { | ||
| return | ||
| } | ||
| if cmd.Annotations == nil { | ||
| cmd.Annotations = map[string]string{} | ||
| } | ||
| cmd.Annotations[riskLevelAnnotationKey] = level | ||
| } | ||
|
|
||
| // GetRisk returns the static risk level. ok is true when the command has a | ||
| // risk annotation. | ||
| func GetRisk(cmd *cobra.Command) (level string, ok bool) { | ||
| if cmd.Annotations == nil { | ||
| return "", false | ||
| } | ||
| level, ok = cmd.Annotations[riskLevelAnnotationKey] | ||
| return level, ok && level != "" | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.