Bundle B CI follow-up: G-3 env-var docs + M-028 closure (final 5 SA1019 sites)

Two CI failures on master after Bundle B merge:

1. Frontend Build / G-3 env-var docs guardrail
   Bundle B introduced CERTCTL_RATE_LIMIT_PER_USER_RPS and
   CERTCTL_RATE_LIMIT_PER_USER_BURST without adding them to
   docs/features.md. The guardrail step that scans Go source for
   getEnv* calls and asserts each appears in a doc page failed.
   Fix: docs/features.md rate-limit section extended with both new
   env vars + a paragraph explaining the per-key keying contract
   from M-025.

2. Go Build & Test / staticcheck SA1019 hits (6 errors)
   The CI workflow runs staticcheck without continue-on-error. Bundle
   7 opened M-028 to track 6 deprecated-API sites; Bundle 9 closed 1
   of them (the elliptic.Marshal in local.go) but kept a deliberate
   regression-oracle reference in bundle9_coverage_test.go protected
   only by golangci-lint's //nolint comment — staticcheck-as-CLI does
   not honor that, only its native //lint:ignore directive.

   Closure of remaining 5 sites:
     cmd/server/main_test.go:47, 163, 192, 465 — 4 × middleware.NewAuth
       migrated to middleware.NewAuthWithNamedKeys with explicit
       NamedAPIKey entries. The auth=none case at line 465 maps to a
       nil NamedAPIKey slice (no-op pass-through, matches the
       NewAuthWithNamedKeys contract for empty input). Audit count was
       3; recon found a 4th at line 465 that was missed.
     internal/api/handler/scep.go:266 — csr.Attributes is a real RFC
       2985 §5.4.1 challengePassword carve-out. Go's stdlib deprecation
       note explicitly applies only to OID 1.2.840.113549.1.9.14
       (requestedExtensions), NOT to OID 1.2.840.113549.1.9.7
       (challengePassword), for which there is no non-deprecated
       stdlib API. Suppressed with native //lint:ignore SA1019 +
       comment block citing the RFC.
     internal/connector/issuer/local/bundle9_coverage_test.go:342 —
       deliberate regression-oracle that calls elliptic.Marshal to
       prove the new crypto/ecdh path is byte-identical. Comment
       converted from //nolint:staticcheck to native //lint:ignore
       SA1019 so staticcheck-as-CLI honors the suppression.

Audit deliverables:
  cowork/comprehensive-audit-2026-04-25/audit-report.md: M-028 box
    flipped [x]; score 30/55 -> 31/55 (Medium 12/27 -> 13/27).
  cowork/comprehensive-audit-2026-04-25/findings.yaml: M-028 status
    partial_closed -> closed with closure note.

Verification:
  go test -count=1 -short ./cmd/server ./internal/api/handler
    ./internal/connector/issuer/local ./internal/api/middleware
    ./internal/config — all green.
  staticcheck on each changed package — 0 SA1019 hits.

Bundle C had M-028 in scope; this CI-fix lift moves it forward so
master CI goes green immediately. Bundle C scope adjusts to remove
M-028 and focuses on M-006 / M-015 / M-016 / M-019 / M-020 plus the
M-007 / M-008 coverage gaps.
This commit is contained in:
shankar0123
2026-04-26 23:35:13 +00:00
parent 1530ff0ee9
commit a172b6ed3b
4 changed files with 37 additions and 19 deletions
+8 -12
View File
@@ -44,9 +44,8 @@ func TestMain_HealthEndpointBypassesAuth(t *testing.T) {
}) })
// Build the handler chain the same way main.go does // Build the handler chain the same way main.go does
authMiddleware := middleware.NewAuth(middleware.AuthConfig{ authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{
Type: "api-key", {Name: "test", Key: "test-secret-key"},
Secret: "test-secret-key",
}) })
// API handler with auth // API handler with auth
@@ -160,9 +159,8 @@ func TestMain_AuthMiddlewareRejectsUnauthorized(t *testing.T) {
}) })
// Wrap with auth middleware // Wrap with auth middleware
authMiddleware := middleware.NewAuth(middleware.AuthConfig{ authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{
Type: "api-key", {Name: "test", Key: "test-secret-key"},
Secret: "test-secret-key",
}) })
chainedHandler := middleware.Chain(protectedHandler, authMiddleware) chainedHandler := middleware.Chain(protectedHandler, authMiddleware)
@@ -189,9 +187,8 @@ func TestMain_AuthMiddlewareAllowsWithValidKey(t *testing.T) {
}) })
// Wrap with auth middleware // Wrap with auth middleware
authMiddleware := middleware.NewAuth(middleware.AuthConfig{ authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{
Type: "api-key", {Name: "test", Key: testKey},
Secret: testKey,
}) })
chainedHandler := middleware.Chain(protectedHandler, authMiddleware) chainedHandler := middleware.Chain(protectedHandler, authMiddleware)
@@ -462,9 +459,8 @@ func TestMain_AuthNoneMode(t *testing.T) {
}) })
// Wrap with auth middleware in "none" mode // Wrap with auth middleware in "none" mode
authMiddleware := middleware.NewAuth(middleware.AuthConfig{ // auth=none equivalent: empty named-keys list is a no-op pass-through.
Type: "none", authMiddleware := middleware.NewAuthWithNamedKeys(nil)
})
chainedHandler := middleware.Chain(protectedHandler, authMiddleware) chainedHandler := middleware.Chain(protectedHandler, authMiddleware)
+11 -2
View File
@@ -60,11 +60,20 @@ Two endpoints are served without auth so the GUI can detect auth mode before log
Token bucket algorithm protecting the control plane from misbehaving clients. Token bucket algorithm protecting the control plane from misbehaving clients.
Bundle B (Audit M-025 / OWASP ASVS L2 §11.2.1): per-key keying. Each
authenticated caller gets a bucket keyed on their API-key name; each
unauthenticated source IP gets its own bucket. Bucket creation is
on-demand under a `sync.RWMutex`; no eviction (the leak is bounded by
realistic operator IP fan-out — appropriate for the OWASP ASVS L2 threat
model of abuse-by-known-clients, not infinite-cardinality scanners).
| Env Var | Default | Description | | Env Var | Default | Description |
|---|---|---| |---|---|---|
| `CERTCTL_RATE_LIMIT_ENABLED` | `true` | Enable/disable | | `CERTCTL_RATE_LIMIT_ENABLED` | `true` | Enable/disable |
| `CERTCTL_RATE_LIMIT_RPS` | `50` | Requests per second | | `CERTCTL_RATE_LIMIT_RPS` | `50` | Per-key requests per second (default applies to IP-keyed buckets; user-keyed buckets fall back to this when `PER_USER_RPS` is unset) |
| `CERTCTL_RATE_LIMIT_BURST` | `100` | Burst capacity | | `CERTCTL_RATE_LIMIT_BURST` | `100` | Per-key burst capacity (default applies to IP-keyed buckets; user-keyed buckets fall back to this when `PER_USER_BURST` is unset) |
| `CERTCTL_RATE_LIMIT_PER_USER_RPS` | `0` | Override RPS for authenticated callers. `0` means "use `RATE_LIMIT_RPS`". Set higher than `RATE_LIMIT_RPS` to grant authenticated clients a more generous budget than anonymous probes. |
| `CERTCTL_RATE_LIMIT_PER_USER_BURST` | `0` | Override burst for authenticated callers. `0` means "use `RATE_LIMIT_BURST`". |
Exceeded requests receive `429 Too Many Requests` with a `Retry-After` header. Exceeded requests receive `429 Too Many Requests` with a `Retry-After` header.
+12
View File
@@ -263,6 +263,18 @@ func extractCSRFields(csrDER []byte) ([]byte, string, string, error) {
// Attributes is []pkix.AttributeTypeAndValueSET where each has Type (OID) // Attributes is []pkix.AttributeTypeAndValueSET where each has Type (OID)
// and Value ([][]pkix.AttributeTypeAndValue). The challenge password value // and Value ([][]pkix.AttributeTypeAndValue). The challenge password value
// is stored as a string in the inner AttributeTypeAndValue.Value field. // is stored as a string in the inner AttributeTypeAndValue.Value field.
//
// Audit M-028 carve-out: Go's stdlib deprecates `csr.Attributes` for the
// specific use case of parsing the "requestedExtensions" CSR attribute
// (OID 1.2.840.113549.1.9.14), pointing callers at `csr.Extensions` /
// `csr.ExtraExtensions`. challengePassword (OID 1.2.840.113549.1.9.7)
// per RFC 2985 §5.4.1 is a SEPARATE CSR attribute that cannot be
// retrieved via Extensions. There is no non-deprecated stdlib API for
// it; callers either accept the deprecation warning or parse the raw
// `csr.RawAttributes` ASN.1 themselves. We accept the warning; the
// staticcheck.conf and golangci-lint rules suppress SA1019 for this
// specific line per the audit closure note.
//lint:ignore SA1019 RFC 2985 challengePassword has no non-deprecated stdlib API; see comment above.
for _, attr := range csr.Attributes { for _, attr := range csr.Attributes {
if attr.Type.Equal(oidChallengePassword) { if attr.Type.Equal(oidChallengePassword) {
if len(attr.Value) > 0 && len(attr.Value[0]) > 0 { if len(attr.Value) > 0 && len(attr.Value[0]) > 0 {
@@ -334,11 +334,12 @@ func TestHashPublicKey_ECDSA_RoundTripPin(t *testing.T) {
t.Fatalf("ecdsaToECDH: %v", err) t.Fatalf("ecdsaToECDH: %v", err)
} }
ecdhBytes := ecdhPub.Bytes() ecdhBytes := ecdhPub.Bytes()
//nolint:staticcheck // SA1019: pin assertion — we DELIBERATELY use // Pin assertion — we DELIBERATELY use the deprecated API here
// the deprecated API here as a regression oracle to prove the // as a regression oracle to prove the new crypto/ecdh path
// new crypto/ecdh path produces byte-identical output. If // produces byte-identical output. If elliptic.Marshal is
// elliptic.Marshal is removed in a future Go release this test // removed in a future Go release this test must be deleted
// must be deleted (and the migration is then irreversibly proven). // (and the migration is then irreversibly proven).
//lint:ignore SA1019 deliberate regression oracle for M-028 round-trip pin
legacy := elliptic.Marshal(k.Curve, k.X, k.Y) legacy := elliptic.Marshal(k.Curve, k.X, k.Y)
if !bytes.Equal(ecdhBytes, legacy) { if !bytes.Equal(ecdhBytes, legacy) {
t.Fatalf("ECDH .Bytes() != legacy elliptic.Marshal output\n new: %x\n old: %x", ecdhBytes, legacy) t.Fatalf("ECDH .Bytes() != legacy elliptic.Marshal output\n new: %x\n old: %x", ecdhBytes, legacy)