AppSec Review of v0.17.0

[jeffmccune]

You are an expert application security consultant reviewing tag v0.17.0. Review the following:

1. Ensure SECURITY.md is correct.
2. Authentication, authorization, and RBAC logic.
3. Audit logging for SOC2 conformance.

---

AppSec Review Plan

Phase 1: SECURITY.md Review

Review SECURITY.md for accuracy against the actual code in console/rpc/auth.go.

Findings:

• Validation flow (8 steps) accurately describes the actual token validation in extractAndVerifyToken() (auth.go:95-137).
• Bearer token extraction code snippets match the source exactly.
• JWT signature verification via verifier.Verify() is correctly documented with OIDC spec references.
• Token expiration, issuer, and audience validation handled by go-oidc library as documented.
• Claims extraction and configurable --roles-claim behavior accurately described.
• ExtractRoles helper behavior (nil for missing claims, skip non-strings) is correct.
• Three interceptor types (LazyAuthInterceptor, AuthInterceptor, OptionalAuthInterceptor) correctly described.
• "Validation NOT Performed" section (nonce, iat, at_hash) is accurate and correctly justified.
• OIDC discovery retry behavior (no error caching) correctly documented.
• TLS enforcement and token storage sections are accurate.

Status: SECURITY.md is correct. No changes needed.

---

Phase 2: Authentication Review

Review console/rpc/auth.go and console/console.go route registration.

Findings:

• All protected services (Secrets, Projects, Organizations) use LazyAuthInterceptor via protectedInterceptors (console.go:205-208).
• Only VersionService and gRPC reflection use publicInterceptors (no auth). Both are intentional per ADR 009 and ADR 010.
• Double-checked locking for lazy OIDC verifier init is correct (auth.go:29-49).
• Token extraction validates: header present, Bearer prefix, non-empty token (auth.go:96-109).
• Claims.Sub fallback to idToken.Subject ensures subject is always populated (auth.go:132-134).
• Custom rolesClaim re-parses raw claims only when needed (auth.go:124-129).

FINDING-01: No-auth fallback when issuer not configured. When --issuer is empty, protectedInterceptors falls back to publicInterceptors (console.go:212), meaning all "protected" endpoints are unauthenticated. This is intentional for development but should be clearly documented.

• Severity: Informational
• Recommendation: Add a slog.Warn at startup when auth is not configured, and document the behavior in SECURITY.md.

FINDING-02: Auth failures not logged. extractAndVerifyToken returns errors for missing/invalid/expired tokens but does not log these events. Failed authentication attempts are invisible in the audit trail.

• Severity: Medium (SOC2 CC6.1 gap)
• Recommendation: Add slog.Warn logging in extractAndVerifyToken for: missing Authorization header, invalid Bearer format, empty token, and verification failures (expired, bad signature, wrong issuer/audience). Include the procedure name and remote address.

---

Phase 3: Authorization & RBAC Review

Review console/rbac/rbac.go, cascade tables, and per-service authorization in console/secrets/, console/organizations/, console/projects/.

Findings:

• Role hierarchy (Viewer < Editor < Owner) with explicit permission maps is correct (rbac.go:50-89).
• CheckAccessGrants uses case-insensitive email/role matching consistently (rbac.go:128-151).
• BestRoleFromGrants correctly returns highest role across all grant sources (rbac.go:173-213).
• ProjectCascadeSecretPerms correctly excludes SecretsRead from cascade (rbac.go:231-245). Reading secret data always requires a direct per-secret grant.
• Organization grants do not cascade to projects or secrets (per ADR 007). Project creation checks org grants explicitly.
• All service handlers check rpc.ClaimsFromContext(ctx) for nil and return CodeUnauthenticated.
• Secrets handler checkAccess() correctly evaluates per-secret grants first, then project cascade grants (secrets/handler.go).
• Organization creation controlled by --disable-org-creation, --org-creator-users, --org-creator-roles flags with correct evaluation logic.
• Creator automatically added as OWNER to newly created organizations via ensureCreatorOwner().
• Time-based grants (Nbf/Exp) filtered by ActiveGrantsMap() before evaluation.

No authorization bypass paths found. RBAC implementation is sound.

---

Phase 4: Audit Logging for SOC2 Conformance

Review structured audit logging across all services and the observability documentation.

Findings:

• Secrets service: Comprehensive audit logging. All CRUD and sharing operations logged at Info level. All denied operations logged at Warn level. Fields include: action, resource_type, secret name, project, sub, email, roles.
• Organizations service: Comprehensive audit logging following the same pattern as secrets.
• Projects service: Comprehensive audit logging following the same pattern as secrets.
• RPC layer: LoggingInterceptor logs all procedure calls with duration. Failures logged at Error level.
• HTTP access log: Apache Combined Log format for all requests (console.go logRequests).

FINDING-03: observability.md only documents secret audit events. Organization and project audit events follow the same pattern but are not documented. Operators reviewing audit capabilities will see an incomplete picture.

• Severity: Low (documentation gap)
• Recommendation: Update docs/observability.md to include organization and project audit event tables.

FINDING-04: HTTP access log lacks authenticated user identity. The HTTP access log records remote_addr, method, path, status, bytes, referer, and user-agent, but does not include the authenticated user's email or subject. Correlating HTTP requests to user identity requires joining with RPC audit logs by timestamp.

• Severity: Low (defense-in-depth)
• Recommendation: Accept risk. RPC-level audit logs already capture user identity for all authenticated operations. Adding user identity to the HTTP access log would require passing claims through the HTTP middleware layer, which adds complexity for marginal benefit.

FINDING-05 (same as FINDING-02): Authentication failures not in audit trail. See Phase 2.

---

Summary

Finding	Severity	Action
FINDING-01: No-auth fallback undocumented	Informational	Add startup warning log and document
FINDING-02: Auth failures not logged	Medium	Add slog.Warn to extractAndVerifyToken
FINDING-03: Incomplete observability docs	Low	Update observability.md
FINDING-04: HTTP log lacks user identity	Low	Accept risk (ADR)

Implementation Plan

• Phase 1: FINDING-02 — Add authentication failure logging to console/rpc/auth.go
• Phase 2: FINDING-01 — Add startup warning when auth is not configured in console/console.go
• Phase 3: FINDING-03 — Update docs/observability.md with org and project audit event tables
• Phase 4: FINDING-04 — Write ADR accepting the risk of HTTP logs without user identity
• Phase 5: Final cleanup — scan for stale docs, dead code, unused imports