From 15db1647e80dbeaa49fd6f9bf28a7a7718071d16 Mon Sep 17 00:00:00 2001 From: Michael Czechowski Date: Sat, 2 May 2026 15:09:36 +0200 Subject: [PATCH] fix(scope): enforce scope violations + WAVE_SKIP_SCOPE_CHECK bypass (#1622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Finding 1 (HIGH): introspection failure → ScopeViolation (was warning+skip) - Finding 2 (MED): nil introspector (unsupported forge) → per-scope violations (was warn+return) - Finding 3 (MED): fine-grained PAT hint updated to reference WAVE_SKIP_SCOPE_CHECK=1 - WAVE_SKIP_SCOPE_CHECK=1 bypass added for fine-grained PATs, unsupported forges, air-gapped envs - Docs: environment.md, manifest.md, concepts/personas.md --- docs/concepts/personas.md | 2 + docs/reference/environment.md | 1 + docs/reference/manifest.md | 6 ++ internal/scope/validator.go | 48 ++++++++++-- internal/scope/validator_test.go | 127 ++++++++++++++++++++++++++++--- 5 files changed, 170 insertions(+), 14 deletions(-) diff --git a/docs/concepts/personas.md b/docs/concepts/personas.md index 8dcaa6eb5..5087c432d 100644 --- a/docs/concepts/personas.md +++ b/docs/concepts/personas.md @@ -313,6 +313,8 @@ personas: Token scopes enforce **least-privilege API access** per persona. During preflight, Wave validates that the active forge token satisfies each persona's declared scopes before pipeline execution begins. This catches misconfigured credentials early rather than failing mid-pipeline. +Introspection failures (including fine-grained GitHub PATs, which lack readable scope headers) produce violations with remediation hints. Set `WAVE_SKIP_SCOPE_CHECK=1` to bypass scope validation when introspection is unavailable. + ### Permission Hierarchy Permissions are hierarchical: `admin` satisfies `write`, which satisfies `read`. Canonical resources include `issues`, `pulls`, `repos`, `actions`, and `packages`. diff --git a/docs/reference/environment.md b/docs/reference/environment.md index 1b59c9666..8e0f6128f 100644 --- a/docs/reference/environment.md +++ b/docs/reference/environment.md @@ -11,6 +11,7 @@ Reference for all environment variables that control Wave behavior, and the cred | `WAVE_MIGRATION_ENABLED` | `bool` | `true` | Enable the database migration system. | | `WAVE_AUTO_MIGRATE` | `bool` | `true` | Automatically apply pending migrations on startup. | | `WAVE_SKIP_MIGRATION_VALIDATION` | `bool` | `false` | Skip migration checksum validation (development only). | +| `WAVE_SKIP_SCOPE_CHECK` | `bool` | `false` | Bypass token scope validation entirely. Use for fine-grained PATs (GitHub), unsupported forges, or air-gapped environments where token introspection is unavailable. | | `WAVE_MAX_MIGRATION_VERSION` | `int` | `0` | Limit migrations to this version (0 = unlimited). Useful for gradual rollout. | | `NO_COLOR` | `string` | _(unset)_ | Disable colored output. Any non-empty value disables color. Follows the [NO_COLOR](https://no-color.org) standard. | diff --git a/docs/reference/manifest.md b/docs/reference/manifest.md index 3fe0cdfc6..6fd38cc4c 100644 --- a/docs/reference/manifest.md +++ b/docs/reference/manifest.md @@ -251,6 +251,12 @@ If `token_scopes` is omitted for a persona, scope validation is skipped for that Unknown resources produce warnings (not errors) to allow forward-compatible scope declarations. +**Introspection failures** (network errors, API errors) produce violations that block execution — the persona explicitly declared required scopes and those cannot be verified. + +**Fine-grained GitHub PATs** lack the `X-OAuth-Scopes` response header used for introspection. Wave surfaces a violation with a remediation hint. Recreate the token as a classic PAT, or set `WAVE_SKIP_SCOPE_CHECK=1` to bypass scope validation for environments where introspection is unavailable. + +**Unsupported forges** (e.g. Bitbucket) produce violations for each declared scope. Set `WAVE_SKIP_SCOPE_CHECK=1` to bypass. + Key sources: `internal/scope/scope.go`, `internal/scope/validator.go`, `internal/scope/resolver.go` ### Temperature Guidelines diff --git a/internal/scope/validator.go b/internal/scope/validator.go index 12d96f739..682ef9d11 100644 --- a/internal/scope/validator.go +++ b/internal/scope/validator.go @@ -2,6 +2,7 @@ package scope import ( "fmt" + "os" "strings" "github.com/recinq/wave/internal/forge" @@ -82,12 +83,35 @@ func defaultTokenEnvVar(ft forge.ForgeType) string { // ValidatePersonas checks all personas' scope requirements against active tokens. // The personas argument maps persona name to its token_scopes slice. // Returns all violations aggregated (FR-006). +// Set WAVE_SKIP_SCOPE_CHECK=1 to bypass all scope validation (fine-grained PATs, air-gapped envs). func (v *Validator) ValidatePersonas(personas map[string][]string) (*ValidationResult, error) { result := &ValidationResult{} - // If no introspector available (unknown forge), warn and skip + if os.Getenv("WAVE_SKIP_SCOPE_CHECK") != "" { + result.Warnings = append(result.Warnings, "WAVE_SKIP_SCOPE_CHECK set; token scope validation bypassed") + return result, nil + } + + // If no introspector available (unsupported forge), emit violations per declared scope if v.introspector == nil { - result.Warnings = append(result.Warnings, fmt.Sprintf("no token introspector available for forge type %q; skipping scope validation", v.forgeInfo.Type)) + for name, tokenScopes := range personas { + for _, scopeStr := range tokenScopes { + ts, _, err := Parse(scopeStr) + if err != nil { + continue + } + envVar := ts.EnvVar + if envVar == "" { + envVar = defaultTokenEnvVar(v.forgeInfo.Type) + } + result.Violations = append(result.Violations, ScopeViolation{ + PersonaName: name, + MissingScope: scopeStr, + EnvVar: envVar, + Hint: fmt.Sprintf("token scope validation not supported for forge %q; set WAVE_SKIP_SCOPE_CHECK=1 to bypass", v.forgeInfo.Type), + }) + } + } return result, nil } @@ -132,7 +156,12 @@ func (v *Validator) ValidatePersonas(personas map[string][]string) (*ValidationR // Resolve abstract scope to platform-specific scopes required, err := v.resolver.Resolve(ts) if err != nil { - result.Warnings = append(result.Warnings, fmt.Sprintf("persona %q: scope resolution for %q: %v", name, scopeStr, err)) + result.Violations = append(result.Violations, ScopeViolation{ + PersonaName: name, + MissingScope: scopeStr, + EnvVar: envVar, + Hint: fmt.Sprintf("scope validation not supported for forge %q; %v", v.forgeInfo.Type, err), + }) continue } @@ -147,9 +176,18 @@ func (v *Validator) ValidatePersonas(personas map[string][]string) (*ValidationR tokenCache[envVar] = tokenInfo } - // If introspection had an error, warn and skip validation for this scope + // If introspection had an error, emit a ScopeViolation instead of warning if tokenInfo.Error != nil { - result.Warnings = append(result.Warnings, fmt.Sprintf("persona %q: token %s introspection: %v", name, envVar, tokenInfo.Error)) + hint := fmt.Sprintf("token introspection failed: %v", tokenInfo.Error) + if tokenInfo.TokenType == "fine-grained" { + hint = "fine-grained PATs cannot be introspected; recreate as classic PAT or set WAVE_SKIP_SCOPE_CHECK=1 to bypass" + } + result.Violations = append(result.Violations, ScopeViolation{ + PersonaName: name, + MissingScope: scopeStr, + EnvVar: envVar, + Hint: hint, + }) continue } diff --git a/internal/scope/validator_test.go b/internal/scope/validator_test.go index 37c49bbe1..8eb3ec2b3 100644 --- a/internal/scope/validator_test.go +++ b/internal/scope/validator_test.go @@ -111,11 +111,18 @@ func TestValidatePersonas_UnknownForge(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if result.HasViolations() { - t.Error("expected no violations for unknown forge (should warn and skip)") + // Unknown forge + nil introspector → violation per declared scope (Finding 2) + if !result.HasViolations() { + t.Error("expected violations for unknown forge (nil introspector)") } - if len(result.Warnings) == 0 { - t.Error("expected warning for unknown forge type") + found := false + for _, v := range result.Violations { + if v.PersonaName == "navigator" && v.MissingScope == "issues:read" { + found = true + } + } + if !found { + t.Error("expected violation for navigator/issues:read on unknown forge") } } @@ -140,12 +147,18 @@ func TestValidatePersonas_IntrospectionFailure(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - // Introspection failure should produce a warning, not a violation - if result.HasViolations() { - t.Error("expected no violations when introspection fails (should warn)") + // Introspection failure should now produce a violation, not just a warning + if !result.HasViolations() { + t.Fatal("expected violation when introspection fails") } - if len(result.Warnings) == 0 { - t.Error("expected warning for introspection failure") + found := false + for _, violation := range result.Violations { + if violation.PersonaName == "navigator" && violation.EnvVar == "GH_TOKEN" { + found = true + } + } + if !found { + t.Error("expected violation for navigator persona with GH_TOKEN") } } @@ -213,3 +226,99 @@ func TestValidatePersonas_EnvPassthroughMissing(t *testing.T) { t.Error("expected violation mentioning GH_TOKEN env_passthrough") } } + +func TestValidatePersonas_BitbucketForgeViolation(t *testing.T) { + resolver := NewResolver(forge.ForgeBitbucket) + introspector := &mockIntrospector{ + results: map[string]*TokenInfo{ + "BITBUCKET_TOKEN": {EnvVar: "BITBUCKET_TOKEN", Scopes: []string{"repo"}, TokenType: "classic"}, + }, + } + v := NewValidator(resolver, introspector, forge.ForgeInfo{Type: forge.ForgeBitbucket}, []string{"BITBUCKET_TOKEN"}) + + personas := map[string][]string{ + "navigator": {"issues:read@BITBUCKET_TOKEN"}, + } + + result, err := v.ValidatePersonas(personas) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Bitbucket should produce a violation, not just a warning + if !result.HasViolations() { + t.Fatal("expected violation for unsupported Bitbucket forge") + } + found := false + for _, violation := range result.Violations { + if violation.PersonaName == "navigator" { + found = true + } + } + if !found { + t.Error("expected violation for navigator persona on Bitbucket") + } +} + +func TestValidatePersonas_FineGrainedPATHint(t *testing.T) { + resolver := NewResolver(forge.ForgeGitHub) + introspector := &mockIntrospector{ + results: map[string]*TokenInfo{ + "GH_TOKEN": { + EnvVar: "GH_TOKEN", + TokenType: "fine-grained", + Error: fmt.Errorf("fine-grained PAT detected; scope introspection not available via headers"), + }, + }, + } + v := NewValidator(resolver, introspector, forge.ForgeInfo{Type: forge.ForgeGitHub}, []string{"GH_TOKEN"}) + + personas := map[string][]string{ + "navigator": {"issues:read"}, + } + + result, err := v.ValidatePersonas(personas) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.HasViolations() { + t.Fatal("expected violation for fine-grained PAT") + } + found := false + for _, violation := range result.Violations { + if violation.PersonaName == "navigator" && + violation.EnvVar == "GH_TOKEN" && + violation.Hint == "fine-grained PATs cannot be introspected; recreate as classic PAT or set WAVE_SKIP_SCOPE_CHECK=1 to bypass" { + found = true + } + } + if !found { + t.Errorf("expected violation with fine-grained PAT remediation hint, got: %+v", result.Violations) + } +} + +func TestValidatePersonas_SkipScopeCheckEnv(t *testing.T) { + t.Setenv("WAVE_SKIP_SCOPE_CHECK", "1") + + resolver := NewResolver(forge.ForgeGitHub) + introspector := &mockIntrospector{ + results: map[string]*TokenInfo{ + "GH_TOKEN": {EnvVar: "GH_TOKEN", Error: fmt.Errorf("introspection failed")}, + }, + } + v := NewValidator(resolver, introspector, forge.ForgeInfo{Type: forge.ForgeGitHub}, []string{"GH_TOKEN"}) + + personas := map[string][]string{ + "navigator": {"issues:read"}, + } + + result, err := v.ValidatePersonas(personas) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.HasViolations() { + t.Error("expected no violations when WAVE_SKIP_SCOPE_CHECK set") + } + if len(result.Warnings) == 0 { + t.Error("expected warning noting bypass is active") + } +}