fix(auth): owner allowlist trusts pass-through-authenticated callers (#59)
Allowlist.Check now takes ctx: when the caller authenticated with their own Gitea PAT (pass-through, v0.12.0), it skips the static GITEA_MCP_ALLOWED_OWNERS check entirely — Gitea's own permission model already gates that caller's access more precisely than a coarse owner-name list can. The static list still applies unchanged for the shared static-token/JWT path, where it's the only defense against the service token's blast radius. Mechanical: every tool call site already had ctx in scope, so this is a signature-only change at 41 call sites, no other tool behavior changes. Closes the "Deferred" item from #59. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,11 @@
|
||||
package allowlist
|
||||
|
||||
import "fmt"
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
|
||||
)
|
||||
|
||||
type Allowlist struct {
|
||||
owners map[string]struct{}
|
||||
@@ -14,10 +19,17 @@ func New(owners []string) *Allowlist {
|
||||
return &Allowlist{owners: m}
|
||||
}
|
||||
|
||||
func (a *Allowlist) Check(owner string) error {
|
||||
// Check gates owner access to the static list — except for a caller
|
||||
// authenticated with their own Gitea PAT (pass-through, gitea-mcp#59), whose
|
||||
// access Gitea's own permission model already gates more precisely than a
|
||||
// coarse owner name list ever could.
|
||||
func (a *Allowlist) Check(ctx context.Context, owner string) error {
|
||||
if owner == "" {
|
||||
return fmt.Errorf("owner required")
|
||||
}
|
||||
if _, ok := gitea.TokenFromContext(ctx); ok {
|
||||
return nil
|
||||
}
|
||||
if _, ok := a.owners[owner]; !ok {
|
||||
return fmt.Errorf("owner %q not in allowlist", owner)
|
||||
}
|
||||
|
||||
@@ -1,16 +1,36 @@
|
||||
package allowlist_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/allowlist"
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestAllowlistCheck(t *testing.T) {
|
||||
a := allowlist.New([]string{"mathias", "acme"})
|
||||
assert.NoError(t, a.Check("mathias"))
|
||||
assert.NoError(t, a.Check("acme"))
|
||||
assert.Error(t, a.Check("evil"))
|
||||
assert.Error(t, a.Check(""))
|
||||
ctx := context.Background()
|
||||
assert.NoError(t, a.Check(ctx, "mathias"))
|
||||
assert.NoError(t, a.Check(ctx, "acme"))
|
||||
assert.Error(t, a.Check(ctx, "evil"))
|
||||
assert.Error(t, a.Check(ctx, ""))
|
||||
}
|
||||
|
||||
// A caller authenticated with their own Gitea PAT (pass-through, gitea-mcp#59)
|
||||
// is gated by Gitea's own permission model, not the MCP's static owner list —
|
||||
// otherwise a legitimate second user could never touch their own repos.
|
||||
func TestAllowlistCheckTrustsPassthroughAuthenticatedCaller(t *testing.T) {
|
||||
a := allowlist.New([]string{"mathias"})
|
||||
ctx := gitea.WithToken(context.Background(), "someone-elses-pat")
|
||||
assert.NoError(t, a.Check(ctx, "someone-else"))
|
||||
}
|
||||
|
||||
// Empty owner is a structural input error, not an authz question — still
|
||||
// rejected even on the pass-through path.
|
||||
func TestAllowlistCheckStillRejectsEmptyOwnerOnPassthrough(t *testing.T) {
|
||||
a := allowlist.New([]string{"mathias"})
|
||||
ctx := gitea.WithToken(context.Background(), "someone-elses-pat")
|
||||
assert.Error(t, a.Check(ctx, ""))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user