Waiting for input...
Star SPIKE on GitHub

ADR-0031: AST-Based Test Enforcement for Route Guard Functions


  • Status: accepted
  • Date: 2024-11-27
  • Tags: Security, Testing, Authorization, Code Quality

Context and Problem Statement

SPIKE Nexus and SPIKE Keeper route handlers must perform authorization checks before processing requests. The established pattern uses net.ReadParseAndGuard which accepts a guard function parameter that is executed internally. However, there is no compile-time or automated mechanism to ensure new route handlers follow this pattern.

A contributor adding a new route could forget to include guard function invocation, creating an authorization bypass vulnerability. Code review catches most issues, but human oversight is fallible.

Decision Drivers

  • Security: Every route must have authorization checks
  • Developer experience: Should not add excessive boilerplate
  • Maintainability: Solution should be self-documenting
  • CI integration: Violations should be caught before merge
  • Flexibility: Must support both standard and custom guard patterns

Considered Options

  1. Mandatory guard parameter on route registration - Pass guard function to all routes, but this does not guarantee the guard is actually called
  2. Wrapper function approach - SecureRoute(pattern, guard, handler) that always calls guard before handler
  3. Interface-based handlers - Require SecureHandler interface with Guard() and Handle() methods
  4. AST-based test - Scan route handler code and verify guard invocation
  5. Convention + code review only - Document the pattern and rely on review

Decision

Implement an AST-based test that scans all route handler files and verifies each Route* function contains a guard invocation.

The test recognizes multiple valid patterns:

  1. net.ReadParseAndGuard calls (standard JSON route pattern)
  2. Functions starting with guard (e.g., guardPolicyDeleteRequest)
  3. Known guarded helper functions (e.g., handleJSONDecrypt for cipher routes that support streaming)

Rationale

  • No code changes required - existing pattern works as-is
  • CI enforcement - test fails if any route lacks guard invocation
  • Self-documenting - test code documents the convention
  • Zero runtime overhead - purely a test-time check
  • Flexible - supports existing cipher routes that use custom guard patterns due to streaming support

The existing net.ReadParseAndGuard pattern already guarantees guard execution for routes that use it. The AST test ensures all routes use either this pattern or an equivalent guard invocation.

Consequences

Positive

  • Authorization bypass vulnerabilities are caught automatically in CI
  • New contributors learn the pattern from test failure messages
  • No changes to production code or runtime behavior
  • Test serves as living documentation of the guard convention

Negative

  • Test must be updated if new guarded helper functions are added
  • AST parsing adds test complexity
  • False positives possible if function naming conventions change

Implementation Notes

SPIKE Nexus

The test is located at app/nexus/internal/route/base/guard_test.go and:

  1. Scans subdirectories: acl/policy, bootstrap, cipher, operator, secret
  2. Finds functions starting with Route
  3. Verifies each calls a guard (via ReadParseAndGuard, guard* functions, or known guarded helpers)
  4. Reports all violations with file paths and function names

Utility files (errors.go, guard.go, handle.go, etc.) are skipped as they do not contain route handlers.

SPIKE Keeper

A similar test is located at app/keeper/internal/route/base/guard_test.go and:

  1. Scans the store subdirectory
  2. Finds functions starting with Route
  3. Verifies each calls a guard (via ReadParseAndGuard or guard* functions)
  4. Reports all violations with file paths and function names

SPIKE Keeper has fewer routes (shard contribution and retrieval) but they are equally critical since they handle root key shards.