From 2e22d0f9acfac90ea8f95f772f59f40d044ddee3 Mon Sep 17 00:00:00 2001 From: Aditya Padmakar Date: Thu, 17 Sep 2026 10:00:50 -0400 Subject: [PATCH] Add NotForCommand helper and fix Exercism plugin auth gating --- plugins/exercism/exercism.go | 5 +- plugins/exercism/exercism_test.go | 77 ++++++++++++++++++++++++++++++ sdk/needsauth/helpers.go | 9 ++++ sdk/needsauth/helpers_test.go | 79 +++++++++++++++++++++++++++---- 4 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 plugins/exercism/exercism_test.go diff --git a/plugins/exercism/exercism.go b/plugins/exercism/exercism.go index b82a799db..d2c93d13b 100644 --- a/plugins/exercism/exercism.go +++ b/plugins/exercism/exercism.go @@ -15,7 +15,10 @@ func ExercismCLI() schema.Executable { NeedsAuth: needsauth.IfAll( needsauth.NotForHelpOrVersion(), needsauth.NotWithoutArgs(), - needsauth.NotForExactArgs("completion", "upgrade", "workspace", "troubleshoot"), + needsauth.NotForCommand("completion"), + needsauth.NotForCommand("upgrade"), + needsauth.NotForCommand("workspace"), + needsauth.NotForCommand("troubleshoot"), ), Uses: []schema.CredentialUsage{ { diff --git a/plugins/exercism/exercism_test.go b/plugins/exercism/exercism_test.go new file mode 100644 index 000000000..769f0198b --- /dev/null +++ b/plugins/exercism/exercism_test.go @@ -0,0 +1,77 @@ +package exercism + +import ( + "testing" + + "github.com/1Password/shell-plugins/sdk/plugintest" +) + +// Regression test for https://github.com/1Password/shell-plugins/issues/619. The Exercism +// subcommands that only touch local setup must not require an Exercism API Key item, both in +// their bare form and with the arguments the CLI accepts for them. +func TestExercismCLINeedsAuth(t *testing.T) { + plugintest.TestNeedsAuth(t, ExercismCLI().NeedsAuth, map[string]plugintest.NeedsAuthCase{ + "no for completion": { + Args: []string{"completion"}, + ExpectedNeedsAuth: false, + }, + "no for completion with shell arg": { + Args: []string{"completion", "bash"}, + ExpectedNeedsAuth: false, + }, + "no for upgrade": { + Args: []string{"upgrade"}, + ExpectedNeedsAuth: false, + }, + "no for workspace": { + Args: []string{"workspace"}, + ExpectedNeedsAuth: false, + }, + "no for troubleshoot": { + Args: []string{"troubleshoot"}, + ExpectedNeedsAuth: false, + }, + "no for troubleshoot with --full-api-key": { + Args: []string{"troubleshoot", "--full-api-key"}, + ExpectedNeedsAuth: false, + }, + "no for workspace with a flag": { + Args: []string{"workspace", "--verbose"}, + ExpectedNeedsAuth: false, + }, + "yes for submit": { + Args: []string{"submit", "bob.go"}, + ExpectedNeedsAuth: true, + }, + "yes for download": { + Args: []string{"download", "--exercise=bob", "--track=go"}, + ExpectedNeedsAuth: true, + }, + "yes for test": { + Args: []string{"test"}, + ExpectedNeedsAuth: true, + }, + // The exempt names are ordinary words that can also appear as exercise names, track + // names, or filenames, so they must only be exempt in command position. + "yes for an exercise named after an exempt subcommand": { + Args: []string{"submit", "completion"}, + ExpectedNeedsAuth: true, + }, + "yes for a subcommand that merely starts with an exempt name": { + Args: []string{"upgrades"}, + ExpectedNeedsAuth: true, + }, + "no without args": { + Args: []string{}, + ExpectedNeedsAuth: false, + }, + "no for --help": { + Args: []string{"--help"}, + ExpectedNeedsAuth: false, + }, + "no for version": { + Args: []string{"version"}, + ExpectedNeedsAuth: false, + }, + }) +} diff --git a/sdk/needsauth/helpers.go b/sdk/needsauth/helpers.go index 39272f7af..65c321a1a 100644 --- a/sdk/needsauth/helpers.go +++ b/sdk/needsauth/helpers.go @@ -51,6 +51,15 @@ func ForCommand(command ...string) sdk.NeedsAuthentication { } } +// NotForCommand returns a NeedsAuthentication rule to opt out of authentication for a +// certain (sub)command and anything nested under it, e.g. ["completion"] or ["config", "get"]. +func NotForCommand(command ...string) sdk.NeedsAuthentication { + forCommand := ForCommand(command...) + return func(in sdk.NeedsAuthenticationInput) bool { + return !forCommand(in) + } +} + // Always returns a NeedsAuthentication rule to always require authentication. func Always() sdk.NeedsAuthentication { return func(in sdk.NeedsAuthenticationInput) bool { diff --git a/sdk/needsauth/helpers_test.go b/sdk/needsauth/helpers_test.go index c5b2e9304..70b0ecefc 100644 --- a/sdk/needsauth/helpers_test.go +++ b/sdk/needsauth/helpers_test.go @@ -87,23 +87,86 @@ func TestContainsArgs(t *testing.T) { } func TestForCommand(t *testing.T) { - plugintest.TestNeedsAuth(t, NotWhenContainsArgs("--mode", "dry-run"), map[string]plugintest.NeedsAuthCase{ - "yes by default": { + plugintest.TestNeedsAuth(t, ForCommand("config"), map[string]plugintest.NeedsAuthCase{ + "no by default": { Args: []string{"deploy"}, + ExpectedNeedsAuth: false, + }, + "no without args": { + Args: []string{}, + ExpectedNeedsAuth: false, + }, + "yes for the bare command": { + Args: []string{"config"}, ExpectedNeedsAuth: true, }, - "yes when only one of the args is present": { - Args: []string{"deploy", "--mode", "live"}, + "yes for a subcommand of the command": { + Args: []string{"config", "get", "token"}, ExpectedNeedsAuth: true, }, - "yes when both args are present, but not in sequence": { - Args: []string{"deploy", "--mode", "live", "--app-name", "dry-run"}, + "yes for the command with flags": { + Args: []string{"config", "--global"}, ExpectedNeedsAuth: true, }, - "no when all args are present in sequence": { - Args: []string{"deploy", "--mode", "dry-run"}, + "no when the command appears after another command": { + Args: []string{"deploy", "config"}, + ExpectedNeedsAuth: false, + }, + "no for a command that merely starts with the same characters": { + Args: []string{"configure"}, + ExpectedNeedsAuth: false, + }, + }) + + plugintest.TestNeedsAuth(t, ForCommand(), map[string]plugintest.NeedsAuthCase{ + "no without a command to match": { + Args: []string{"config"}, + ExpectedNeedsAuth: false, + }, + }) +} + +func TestForNestedCommand(t *testing.T) { + plugintest.TestNeedsAuth(t, ForCommand("shell-completions", "install"), map[string]plugintest.NeedsAuthCase{ + "yes for the exact nested command": { + Args: []string{"shell-completions", "install"}, + ExpectedNeedsAuth: true, + }, + "yes for the nested command with args": { + Args: []string{"shell-completions", "install", "--shell", "bash"}, + ExpectedNeedsAuth: true, + }, + "no for only the first part of the nested command": { + Args: []string{"shell-completions"}, ExpectedNeedsAuth: false, }, + "no for the nested command's parts in the wrong order": { + Args: []string{"install", "shell-completions"}, + ExpectedNeedsAuth: false, + }, + }) +} + +// NotForCommand negates ForCommand, so the prefix matching itself is covered above. What is +// worth pinning here is the inversion, and that NotForCommand() opts out of nothing, where +// NotForExactArgs() matches the empty arg list and is aliased as NotWithoutArgs(). +func TestNotForCommand(t *testing.T) { + plugintest.TestNeedsAuth(t, NotForCommand("config"), map[string]plugintest.NeedsAuthCase{ + "no for the command and anything nested under it": { + Args: []string{"config", "get", "token"}, + ExpectedNeedsAuth: false, + }, + "yes when the command is not in command position": { + Args: []string{"deploy", "config"}, + ExpectedNeedsAuth: true, + }, + }) + + plugintest.TestNeedsAuth(t, NotForCommand(), map[string]plugintest.NeedsAuthCase{ + "yes without a command to opt out of": { + Args: []string{"config"}, + ExpectedNeedsAuth: true, + }, }) }