From 73c8d7d22096867f3c9890f1a2f6238cfbc74b5a Mon Sep 17 00:00:00 2001 From: llin2 Date: Wed, 23 Sep 2026 23:17:39 +0000 Subject: [PATCH 1/3] feat: restrict task invocation by caller Co-authored-by: Cursor Branch-Creation-Time: 2026-09-23T23:12:07+0000 --- SPECIFICATION.md | 49 ++++++ docs/how-to/create-tasks.md | 39 +++++ docs/reference/cli.md | 37 +++- main.go | 20 ++- pkg/codingcontext/context.go | 18 ++ pkg/codingcontext/invocation.go | 71 ++++++++ pkg/codingcontext/invocation_test.go | 158 ++++++++++++++++++ pkg/codingcontext/markdown/frontmatter.go | 8 + .../markdown/frontmatter_task_test.go | 34 +++- pkg/codingcontext/options.go | 17 +- 10 files changed, 443 insertions(+), 8 deletions(-) create mode 100644 pkg/codingcontext/invocation.go create mode 100644 pkg/codingcontext/invocation_test.go diff --git a/SPECIFICATION.md b/SPECIFICATION.md index df0e63b6..a98d4072 100644 --- a/SPECIFICATION.md +++ b/SPECIFICATION.md @@ -581,6 +581,43 @@ expand: false --- ``` +#### 5.2.13 `allowed_surfaces` (optional) +- **Type:** Array of strings +- **Purpose:** Restrict task execution to matching invocation surfaces +- **Default:** Unrestricted when omitted or empty +- **Matching:** Case-insensitive OR logic across entries + +```yaml +--- +allowed_surfaces: + - jira + - github +--- +``` + +The caller must configure its invocation surface with `WithSurface` or the +`-surface` CLI flag. A restricted task rejects invocations with a missing or +non-matching surface before task content is assembled. + +#### 5.2.14 `allowed_requesters` (optional) +- **Type:** Array of strings +- **Purpose:** Restrict task execution to matching requester usernames or email addresses +- **Default:** Unrestricted when omitted or empty +- **Matching:** Case-insensitive OR logic across entries + +```yaml +--- +allowed_requesters: + - alice + - alice@example.com +--- +``` + +The caller must configure a requester with `WithCaller` or the `-caller` and +`-caller-email` CLI flags. A username or email match permits the requester. +When both `allowed_surfaces` and `allowed_requesters` are present, both +restrictions must match. + ### 5.3 Standard Rule Fields #### 5.3.1 `id` (optional) @@ -1554,6 +1591,16 @@ Potential future additions while maintaining backward compatibility: "description": "Enable/disable parameter expansion", "default": true }, + "allowed_surfaces": { + "type": "array", + "items": {"type": "string"}, + "description": "Invocation surfaces permitted to run a task" + }, + "allowed_requesters": { + "type": "array", + "items": {"type": "string"}, + "description": "Requester usernames or email addresses permitted to run a task" + }, "selectors": { "type": "object", "description": "Auto-filter rules", @@ -1634,6 +1681,8 @@ Potential future additions while maintaining backward compatibility: | `timeout` | string | No | Timeout duration | | `selectors` | object | No | Auto-filter rules | | `expand` | boolean | No | Parameter expansion control | +| `allowed_surfaces` | array | No | Permitted invocation surfaces | +| `allowed_requesters` | array | No | Permitted requester usernames or email addresses | ### Rule Fields diff --git a/docs/how-to/create-tasks.md b/docs/how-to/create-tasks.md index 0c580830..ddc8f10d 100644 --- a/docs/how-to/create-tasks.md +++ b/docs/how-to/create-tasks.md @@ -169,6 +169,45 @@ coding-context -s priority=high implement-feature **Note:** Language values should be lowercase (e.g., `go`, `python`, `javascript`). +## Restricting Task Invocation + +Use `allowed_surfaces` and `allowed_requesters` to limit who may run a task and +where the invocation may originate: + +```markdown +--- +allowed_surfaces: + - jira + - github +allowed_requesters: + - alice + - alice@example.com +--- +# Deploy Service + +Deploy the requested service update. +``` + +Values within each list use case-insensitive OR logic. When both fields are +present, the invocation must match both restrictions. Omitting a field or +setting it to an empty list leaves that dimension unrestricted. + +Library callers provide trusted invocation data with `WithSurface` and +`WithCaller`. Direct CLI callers use `-surface`, `-caller`, and optionally +`-caller-email`: + +```bash +coding-context \ + -surface jira \ + -caller alice \ + -caller-email alice@example.com \ + deploy-service +``` + +Restricted tasks reject missing or non-matching invocation data before task +content is assembled. Linting validates the task file without enforcing +invocation restrictions. + ## Task Frontmatter Task frontmatter is used for filtering and metadata purposes but is **NOT included in the output**. The frontmatter controls behavior (like auto-filtering rules via the `selectors` field) but only the task content below the frontmatter delimiters is included in the assembled context. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 18ebfd14..688f1998 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -184,7 +184,7 @@ coding-context \ ### `-a ` -**Type:** String +**Type:** String **Default:** (empty) Specify the target agent being used. This is currently used for: @@ -216,6 +216,41 @@ coding-context -a copilot -w implement-feature **See also:** [Targeting a Specific Agent](../../README.md#targeting-a-specific-agent) in README +### `-surface ` + +**Type:** String +**Default:** (empty) + +Set the invocation surface used to authorize a task that declares +`allowed_surfaces`. Matching is case-insensitive. A restricted task rejects an +empty or non-matching surface. + +### `-caller ` + +**Type:** String +**Default:** (empty) + +Set the requester username used to authorize a task that declares +`allowed_requesters`. + +### `-caller-email ` + +**Type:** String +**Default:** (empty) + +Set the requester email address used to authorize a task that declares +`allowed_requesters`. Either the configured username or email may match an +allowed requester. + +**Example:** +```bash +coding-context \ + -surface jira \ + -caller alice \ + -caller-email alice@example.com \ + deploy-service +``` + ### `-r` **Type:** Boolean flag diff --git a/main.go b/main.go index 8c214352..1ff5f374 100644 --- a/main.go +++ b/main.go @@ -18,11 +18,11 @@ import ( ) var ( - errInvalidUsage = errors.New("invalid usage: expected one task name argument and optional user-prompt") - errWriteRulesNoAgent = errors.New("-w flag requires an agent to be specified (via task 'agent' field or -a flag)") - errNoUserRulePath = errors.New("no user rule path available for agent") - errRulesPathEscapesHome = errors.New("rules path escapes home directory") - errAgentFlagsMutExcl = errors.New("-a and -A flags are mutually exclusive") + errInvalidUsage = errors.New("invalid usage: expected one task name argument and optional user-prompt") + errWriteRulesNoAgent = errors.New("-w flag requires an agent to be specified (via task 'agent' field or -a flag)") + errNoUserRulePath = errors.New("no user rule path available for agent") + errRulesPathEscapesHome = errors.New("rules path escapes home directory") + errAgentFlagsMutExcl = errors.New("-a and -A flags are mutually exclusive") ) type cliConfig struct { @@ -37,6 +37,8 @@ type cliConfig struct { searchPaths []string lenientSearchPaths []string manifestURL string + surface string + caller codingcontext.CallerIdentity taskName string userPrompt string } @@ -81,6 +83,8 @@ func run(ctx context.Context, logger *slog.Logger) error { codingcontext.WithUserPrompt(cfg.userPrompt), codingcontext.WithAgent(cfg.agent), codingcontext.WithLenientAgent(cfg.lenientAgent), + codingcontext.WithSurface(cfg.surface), + codingcontext.WithCaller(cfg.caller), ) result, err := cc.Run(ctx, cfg.taskName) @@ -156,6 +160,12 @@ func parseFlags(logger *slog.Logger) (*cliConfig, error) { }) flag.StringVar(&cfg.manifestURL, "m", "", "Go Getter URL to a manifest file containing search paths (one per line). Every line is included as-is.") + flag.StringVar(&cfg.surface, "surface", "", + "Invocation surface used to authorize tasks that declare allowed_surfaces.") + flag.StringVar(&cfg.caller.Username, "caller", "", + "Requester username used to authorize tasks that declare allowed_requesters.") + flag.StringVar(&cfg.caller.Email, "caller-email", "", + "Requester email used to authorize tasks that declare allowed_requesters.") setupUsage(logger) flag.Parse() diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index 3e24192e..41c18321 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -40,6 +40,8 @@ var ( // ErrMultipleAgents is returned when more than one agent option (WithAgent, WithLenientAgent) is used. // These options are mutually exclusive; only one agent may be set. ErrMultipleAgents = errors.New("only one agent option (WithAgent or WithLenientAgent) may be used") + // ErrCallerNotAllowed is returned when a task does not permit the configured invocation surface or caller. + ErrCallerNotAllowed = errors.New("task invocation not allowed") // ErrInvalidTaskNameNamespace is returned when the task name has an empty namespace. ErrInvalidTaskNameNamespace = errors.New("namespace must not be empty") @@ -83,6 +85,8 @@ type Context struct { agentSetCount int // Incremented by WithAgent and WithLenientAgent; >1 means conflict namespace string // Active namespace derived from task name (e.g. "myteam" from "myteam/fix-bug") userPrompt string // User-provided prompt to append to task + surface string + caller CallerIdentity lintMode bool lintCollector *lintCollector } @@ -197,6 +201,10 @@ func (cc *Context) Run(ctx context.Context, taskName string) (*Result, error) { // Get the task by name if err := cc.findTask(taskName); err != nil { + if errors.Is(err, ErrCallerNotAllowed) { + return nil, err + } + return nil, fmt.Errorf("task not found: %w", err) } @@ -275,6 +283,10 @@ func (cc *Context) visitMarkdownFiles(searchDirFn func(path string) []string, vi for _, dir := range searchDirs { if err := cc.visitMarkdownInDir(dir.path, visitor); err != nil { + if errors.Is(err, ErrCallerNotAllowed) { + return err + } + if dir.lenient { cc.logger.Warn("skipping directory", "path", dir.path, "error", err) @@ -421,6 +433,12 @@ func (cc *Context) loadTask(path, taskName string) error { frontMatter.Name = nameFromPath(path) } + if !cc.lintMode { + if err := cc.authorizeTaskInvocation(frontMatter); err != nil { + return err + } + } + // Extract selector labels from task frontmatter and add them to cc.includes. // This combines CLI selectors (from -s flag) with task selectors using OR logic: // rules match if their frontmatter value matches ANY selector value for a given key. diff --git a/pkg/codingcontext/invocation.go b/pkg/codingcontext/invocation.go new file mode 100644 index 00000000..3ec2f27c --- /dev/null +++ b/pkg/codingcontext/invocation.go @@ -0,0 +1,71 @@ +package codingcontext + +import ( + "fmt" + "strings" + + "github.com/kitproj/coding-context-cli/pkg/codingcontext/markdown" +) + +// CallerIdentity identifies the requester invoking a task. +type CallerIdentity struct { + Username string + Email string +} + +func (cc *Context) authorizeTaskInvocation(frontMatter markdown.TaskFrontMatter) error { + if len(frontMatter.AllowedSurfaces) > 0 { + if strings.TrimSpace(cc.surface) == "" { + return fmt.Errorf("%w: surface is required by the task allowlist", ErrCallerNotAllowed) + } + + if !matchesAllowedValue(frontMatter.AllowedSurfaces, cc.surface) { + return fmt.Errorf("%w: surface %q is not permitted", ErrCallerNotAllowed, cc.surface) + } + } + + if len(frontMatter.AllowedRequesters) == 0 { + return nil + } + + if strings.TrimSpace(cc.caller.Username) == "" && strings.TrimSpace(cc.caller.Email) == "" { + return fmt.Errorf("%w: requester identity is required by the task allowlist", ErrCallerNotAllowed) + } + + if matchesAllowedValue(frontMatter.AllowedRequesters, cc.caller.Username, cc.caller.Email) { + return nil + } + + return fmt.Errorf("%w: requester %s is not permitted", ErrCallerNotAllowed, cc.caller.description()) +} + +func matchesAllowedValue(allowed []string, actualValues ...string) bool { + for _, actual := range actualValues { + actual = strings.TrimSpace(actual) + if actual == "" { + continue + } + + for _, candidate := range allowed { + if strings.EqualFold(actual, strings.TrimSpace(candidate)) { + return true + } + } + } + + return false +} + +func (c CallerIdentity) description() string { + username := strings.TrimSpace(c.Username) + email := strings.TrimSpace(c.Email) + + switch { + case username != "" && email != "": + return fmt.Sprintf("%q <%s>", username, email) + case username != "": + return fmt.Sprintf("%q", username) + default: + return fmt.Sprintf("%q", email) + } +} diff --git a/pkg/codingcontext/invocation_test.go b/pkg/codingcontext/invocation_test.go new file mode 100644 index 00000000..8e4b28ea --- /dev/null +++ b/pkg/codingcontext/invocation_test.go @@ -0,0 +1,158 @@ +package codingcontext + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRun_TaskInvocationAuthorization(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + frontmatter string + opts []Option + wantErr bool + }{ + { + name: "unrestricted task", + }, + { + name: "empty allowlists leave task unrestricted", + frontmatter: `allowed_surfaces: [] +allowed_requesters: []`, + }, + { + name: "surface matches case insensitively", + frontmatter: "allowed_surfaces: [jira, github]", + opts: []Option{WithSurface(" JIRA ")}, + }, + { + name: "requester username matches", + frontmatter: "allowed_requesters: [alice]", + opts: []Option{WithCaller(CallerIdentity{ + Username: "ALICE", + Email: "other@example.com", + })}, + }, + { + name: "requester email matches", + frontmatter: "allowed_requesters: [alice@example.com]", + opts: []Option{WithCaller(CallerIdentity{ + Username: "other", + Email: "Alice@Example.com", + })}, + }, + { + name: "surface and requester both match", + frontmatter: `allowed_surfaces: [github] +allowed_requesters: [alice, alice@example.com]`, + opts: []Option{ + WithSurface("github"), + WithCaller(CallerIdentity{Username: "alice"}), + }, + }, + { + name: "surface mismatch", + frontmatter: "allowed_surfaces: [jira]", + opts: []Option{WithSurface("github")}, + wantErr: true, + }, + { + name: "surface missing", + frontmatter: "allowed_surfaces: [jira]", + wantErr: true, + }, + { + name: "requester mismatch", + frontmatter: "allowed_requesters: [alice]", + opts: []Option{WithCaller(CallerIdentity{Username: "bob", Email: "bob@example.com"})}, + wantErr: true, + }, + { + name: "requester missing", + frontmatter: "allowed_requesters: [alice]", + wantErr: true, + }, + { + name: "surface matches but requester does not", + frontmatter: `allowed_surfaces: [jira] +allowed_requesters: [alice]`, + opts: []Option{ + WithSurface("jira"), + WithCaller(CallerIdentity{Username: "bob"}), + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "restricted", tt.frontmatter, "Authorized task.") + + opts := append([]Option{WithSearchPaths(dir)}, tt.opts...) + result, err := New(opts...).Run(context.Background(), "restricted") + + if tt.wantErr { + require.ErrorIs(t, err, ErrCallerNotAllowed) + require.Nil(t, result) + + return + } + + require.NoError(t, err) + require.Contains(t, result.Prompt, "Authorized task.") + }) + } +} + +func TestRun_AuthorizationPrecedesTaskRendering(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "restricted", "allowed_surfaces: [jira]", "/missing-command") + + result, err := New( + WithSearchPaths(dir), + WithSurface("github"), + ).Run(context.Background(), "restricted") + + require.ErrorIs(t, err, ErrCallerNotAllowed) + require.Nil(t, result) + require.NotErrorIs(t, err, ErrCommandNotFound) +} + +func TestRun_LenientSearchPathDoesNotSuppressAuthorizationDenial(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "restricted", "allowed_surfaces: [jira]", "Restricted task.") + + result, err := New( + WithLenientSearchPaths(dir), + WithSurface("github"), + ).Run(context.Background(), "restricted") + + require.ErrorIs(t, err, ErrCallerNotAllowed) + require.Nil(t, result) +} + +func TestLint_RestrictedTaskDoesNotRequireInvocationIdentity(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "restricted", `allowed_surfaces: [jira] +allowed_requesters: [alice]`, "Lint this task.") + + result, err := New(WithSearchPaths(dir)).Lint(context.Background(), "restricted") + + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, []string{"jira"}, result.Task.FrontMatter.AllowedSurfaces) + require.Equal(t, []string{"alice"}, result.Task.FrontMatter.AllowedRequesters) +} diff --git a/pkg/codingcontext/markdown/frontmatter.go b/pkg/codingcontext/markdown/frontmatter.go index a723582b..eb92e6b2 100644 --- a/pkg/codingcontext/markdown/frontmatter.go +++ b/pkg/codingcontext/markdown/frontmatter.go @@ -63,6 +63,14 @@ type TaskFrontMatter struct { // any active selector are included by default. Defaults to true (current behaviour). // Set to false to require an explicit selector match (strict/opt-in mode). IncludeUnmatched *bool `json:"include_unmatched,omitempty" yaml:"include_unmatched,omitempty"` + + // AllowedSurfaces restricts which invocation surfaces may run the task. + // An empty list leaves the task unrestricted by surface. + AllowedSurfaces []string `json:"allowed_surfaces,omitempty" yaml:"allowed_surfaces,omitempty"` + + // AllowedRequesters restricts which requester usernames or email addresses may run the task. + // An empty list leaves the task unrestricted by requester. + AllowedRequesters []string `json:"allowed_requesters,omitempty" yaml:"allowed_requesters,omitempty"` } // populateContent unmarshals raw JSON into the inline Content map. diff --git a/pkg/codingcontext/markdown/frontmatter_task_test.go b/pkg/codingcontext/markdown/frontmatter_task_test.go index 353f1c4e..075cdaca 100644 --- a/pkg/codingcontext/markdown/frontmatter_task_test.go +++ b/pkg/codingcontext/markdown/frontmatter_task_test.go @@ -2,6 +2,7 @@ package markdown import ( "encoding/json" + "slices" "testing" yaml "github.com/goccy/go-yaml" @@ -48,12 +49,21 @@ func TestTaskFrontMatter_Marshal(t *testing.T) { Model: "gpt-4", SingleShot: true, Timeout: "10m", + AllowedSurfaces: []string{ + "jira", + "github", + }, + AllowedRequesters: []string{ + "alice", + "alice@example.com", + }, Selectors: map[string]any{ "stage": "implementation", }, }, want: "name: Full Task\ndescription: A task with all fields\ntask_name: full-task\nagent: cursor\n" + - "languages:\n- go\nmodel: gpt-4\nsingle_shot: true\ntimeout: 10m\nselectors:\n stage: implementation\n", + "languages:\n- go\nmodel: gpt-4\nsingle_shot: true\ntimeout: 10m\nselectors:\n stage: implementation\n" + + "allowed_surfaces:\n- jira\n- github\nallowed_requesters:\n- alice\n- alice@example.com\n", }, { name: "task with multiple languages", @@ -172,6 +182,12 @@ single_shot: true timeout: 10m selectors: stage: implementation +allowed_surfaces: + - jira + - github +allowed_requesters: + - alice + - alice@example.com `, want: TaskFrontMatter{ BaseFrontMatter: BaseFrontMatter{ @@ -187,6 +203,14 @@ selectors: Model: "gpt-4", SingleShot: true, Timeout: "10m", + AllowedSurfaces: []string{ + "jira", + "github", + }, + AllowedRequesters: []string{ + "alice", + "alice@example.com", + }, Selectors: map[string]any{ "stage": "implementation", }, @@ -253,6 +277,14 @@ func assertTaskFrontMatter(t *testing.T, got, want TaskFrontMatter, err error, w t.Errorf("Timeout = %q, want %q", got.Timeout, want.Timeout) } + if !slices.Equal(got.AllowedSurfaces, want.AllowedSurfaces) { + t.Errorf("AllowedSurfaces = %v, want %v", got.AllowedSurfaces, want.AllowedSurfaces) + } + + if !slices.Equal(got.AllowedRequesters, want.AllowedRequesters) { + t.Errorf("AllowedRequesters = %v, want %v", got.AllowedRequesters, want.AllowedRequesters) + } + switch { case got.IncludeUnmatched == nil && want.IncludeUnmatched == nil: // both unset — ok diff --git a/pkg/codingcontext/options.go b/pkg/codingcontext/options.go index 59413cb5..d5602c9c 100644 --- a/pkg/codingcontext/options.go +++ b/pkg/codingcontext/options.go @@ -114,9 +114,24 @@ func WithUserPrompt(userPrompt string) Option { } } +// WithSurface sets the surface from which the task invocation originated. +func WithSurface(surface string) Option { + return func(c *Context) { + c.surface = surface + } +} + +// WithCaller sets the identity of the requester invoking the task. +func WithCaller(caller CallerIdentity) Option { + return func(c *Context) { + c.caller = caller + } +} + // WithLint enables lint mode: skips bootstrap script execution and shell command // expansion (!`cmd`). File access is tracked and non-fatal structural errors are -// collected in LintResult. Use Lint() instead of Run() to retrieve results. +// collected in LintResult. Caller allowlists are parsed but not enforced. +// Use Lint() instead of Run() to retrieve results. func WithLint(lint bool) Option { return func(c *Context) { c.lintMode = lint From e1709edbdf61bafcf1f54e0695cbcf2ce9dcc49a Mon Sep 17 00:00:00 2001 From: llin2 Date: Thu, 24 Sep 2026 01:14:06 +0000 Subject: [PATCH 2/3] refactor: simplify task invocation authorization Co-authored-by: Cursor --- pkg/codingcontext/authorization.go | 64 +++++++++++++++++ ...vocation_test.go => authorization_test.go} | 0 pkg/codingcontext/context.go | 16 ++--- pkg/codingcontext/invocation.go | 71 ------------------- 4 files changed, 68 insertions(+), 83 deletions(-) create mode 100644 pkg/codingcontext/authorization.go rename pkg/codingcontext/{invocation_test.go => authorization_test.go} (100%) delete mode 100644 pkg/codingcontext/invocation.go diff --git a/pkg/codingcontext/authorization.go b/pkg/codingcontext/authorization.go new file mode 100644 index 00000000..841ecbe9 --- /dev/null +++ b/pkg/codingcontext/authorization.go @@ -0,0 +1,64 @@ +package codingcontext + +import ( + "fmt" + "strings" + + "github.com/kitproj/coding-context-cli/pkg/codingcontext/markdown" +) + +// CallerIdentity identifies the requester invoking a task. +type CallerIdentity struct { + Username string + Email string +} + +func (cc *Context) authorizeTaskInvocation(frontMatter markdown.TaskFrontMatter) error { + if err := authorizeSurface(frontMatter.AllowedSurfaces, cc.surface); err != nil { + return err + } + + return authorizeRequester(frontMatter.AllowedRequesters, cc.caller) +} + +func authorizeSurface(allowed []string, surface string) error { + if len(allowed) == 0 { + return nil + } + + surface = strings.TrimSpace(surface) + if matchesAllowedValue(allowed, surface) { + return nil + } + + return fmt.Errorf("%w: surface %q is not permitted", ErrCallerNotAllowed, surface) +} + +func authorizeRequester(allowed []string, caller CallerIdentity) error { + if len(allowed) == 0 { + return nil + } + + if !matchesAllowedValue(allowed, caller.Username, caller.Email) { + return fmt.Errorf("%w: requester identity is not permitted", ErrCallerNotAllowed) + } + + return nil +} + +func matchesAllowedValue(allowed []string, actualValues ...string) bool { + for _, actual := range actualValues { + actual = strings.TrimSpace(actual) + if actual == "" { + continue + } + + for _, candidate := range allowed { + if strings.EqualFold(actual, strings.TrimSpace(candidate)) { + return true + } + } + } + + return false +} diff --git a/pkg/codingcontext/invocation_test.go b/pkg/codingcontext/authorization_test.go similarity index 100% rename from pkg/codingcontext/invocation_test.go rename to pkg/codingcontext/authorization_test.go diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index 41c18321..1493c5c7 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -201,11 +201,7 @@ func (cc *Context) Run(ctx context.Context, taskName string) (*Result, error) { // Get the task by name if err := cc.findTask(taskName); err != nil { - if errors.Is(err, ErrCallerNotAllowed) { - return nil, err - } - - return nil, fmt.Errorf("task not found: %w", err) + return nil, fmt.Errorf("find task %q: %w", taskName, err) } // Log parameters and selectors after task is found @@ -283,17 +279,13 @@ func (cc *Context) visitMarkdownFiles(searchDirFn func(path string) []string, vi for _, dir := range searchDirs { if err := cc.visitMarkdownInDir(dir.path, visitor); err != nil { - if errors.Is(err, ErrCallerNotAllowed) { - return err - } - - if dir.lenient { + if dir.lenient && !errors.Is(err, ErrCallerNotAllowed) { cc.logger.Warn("skipping directory", "path", dir.path, "error", err) continue } - return err + return fmt.Errorf("process markdown search path %q: %w", dir.path, err) } } @@ -435,7 +427,7 @@ func (cc *Context) loadTask(path, taskName string) error { if !cc.lintMode { if err := cc.authorizeTaskInvocation(frontMatter); err != nil { - return err + return fmt.Errorf("authorize task %q: %w", taskName, err) } } diff --git a/pkg/codingcontext/invocation.go b/pkg/codingcontext/invocation.go deleted file mode 100644 index 3ec2f27c..00000000 --- a/pkg/codingcontext/invocation.go +++ /dev/null @@ -1,71 +0,0 @@ -package codingcontext - -import ( - "fmt" - "strings" - - "github.com/kitproj/coding-context-cli/pkg/codingcontext/markdown" -) - -// CallerIdentity identifies the requester invoking a task. -type CallerIdentity struct { - Username string - Email string -} - -func (cc *Context) authorizeTaskInvocation(frontMatter markdown.TaskFrontMatter) error { - if len(frontMatter.AllowedSurfaces) > 0 { - if strings.TrimSpace(cc.surface) == "" { - return fmt.Errorf("%w: surface is required by the task allowlist", ErrCallerNotAllowed) - } - - if !matchesAllowedValue(frontMatter.AllowedSurfaces, cc.surface) { - return fmt.Errorf("%w: surface %q is not permitted", ErrCallerNotAllowed, cc.surface) - } - } - - if len(frontMatter.AllowedRequesters) == 0 { - return nil - } - - if strings.TrimSpace(cc.caller.Username) == "" && strings.TrimSpace(cc.caller.Email) == "" { - return fmt.Errorf("%w: requester identity is required by the task allowlist", ErrCallerNotAllowed) - } - - if matchesAllowedValue(frontMatter.AllowedRequesters, cc.caller.Username, cc.caller.Email) { - return nil - } - - return fmt.Errorf("%w: requester %s is not permitted", ErrCallerNotAllowed, cc.caller.description()) -} - -func matchesAllowedValue(allowed []string, actualValues ...string) bool { - for _, actual := range actualValues { - actual = strings.TrimSpace(actual) - if actual == "" { - continue - } - - for _, candidate := range allowed { - if strings.EqualFold(actual, strings.TrimSpace(candidate)) { - return true - } - } - } - - return false -} - -func (c CallerIdentity) description() string { - username := strings.TrimSpace(c.Username) - email := strings.TrimSpace(c.Email) - - switch { - case username != "" && email != "": - return fmt.Sprintf("%q <%s>", username, email) - case username != "": - return fmt.Sprintf("%q", username) - default: - return fmt.Sprintf("%q", email) - } -} From 88f9e12b31d0e07d250bc9d36c4a8d474dd5fc95 Mon Sep 17 00:00:00 2001 From: llin2 Date: Thu, 24 Sep 2026 16:53:26 +0000 Subject: [PATCH 3/3] refactor: simplify allowlist authorization Co-authored-by: Cursor --- pkg/codingcontext/authorization.go | 43 +++++++++--------------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/pkg/codingcontext/authorization.go b/pkg/codingcontext/authorization.go index 841ecbe9..35939be5 100644 --- a/pkg/codingcontext/authorization.go +++ b/pkg/codingcontext/authorization.go @@ -14,47 +14,30 @@ type CallerIdentity struct { } func (cc *Context) authorizeTaskInvocation(frontMatter markdown.TaskFrontMatter) error { - if err := authorizeSurface(frontMatter.AllowedSurfaces, cc.surface); err != nil { - return err + if !isAllowed(frontMatter.AllowedSurfaces, cc.surface) { + return fmt.Errorf("%w: surface is not permitted", ErrCallerNotAllowed) } - return authorizeRequester(frontMatter.AllowedRequesters, cc.caller) -} - -func authorizeSurface(allowed []string, surface string) error { - if len(allowed) == 0 { - return nil - } - - surface = strings.TrimSpace(surface) - if matchesAllowedValue(allowed, surface) { - return nil - } - - return fmt.Errorf("%w: surface %q is not permitted", ErrCallerNotAllowed, surface) -} - -func authorizeRequester(allowed []string, caller CallerIdentity) error { - if len(allowed) == 0 { - return nil - } - - if !matchesAllowedValue(allowed, caller.Username, caller.Email) { + if !isAllowed(frontMatter.AllowedRequesters, cc.caller.Username, cc.caller.Email) { return fmt.Errorf("%w: requester identity is not permitted", ErrCallerNotAllowed) } return nil } -func matchesAllowedValue(allowed []string, actualValues ...string) bool { - for _, actual := range actualValues { - actual = strings.TrimSpace(actual) - if actual == "" { +func isAllowed(allowlist []string, values ...string) bool { + if len(allowlist) == 0 { + return true + } + + for _, value := range values { + value = strings.TrimSpace(value) + if value == "" { continue } - for _, candidate := range allowed { - if strings.EqualFold(actual, strings.TrimSpace(candidate)) { + for _, allowed := range allowlist { + if strings.EqualFold(value, strings.TrimSpace(allowed)) { return true } }