mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:11:30 +00:00
596e675ec7
Bundle 5 closure (2026-05-13 acquisition diligence audit). 13-finding
security audit pass across the auth / OIDC / MCP / API / browser-
security surface. Five real closures shipped in code, two false-as-
stated findings annotated with the existing implementation, three
operator-decision items documented for v3 follow-up, three doc-only
fixes (auth architecture narrative aligned with shipped OIDC).
Source findings closed (code):
S1 break-glass /auth/breakglass/login lacked the documented
5/min per-source-IP rate limit; handler now owns its own
SlidingWindowLimiter wired at startup. Doc claim turns true.
R6 OIDC test_discovery JWKS probe ran on http.DefaultClient;
now uses an http.Client whose transport wraps
validation.SafeHTTPDialContext. JWKS URI can no longer
pivot into reserved-address ranges via DNS rebinding.
R7 Slack + Teams notifiers built http.Client without the SSRF
dial-time guard. Both New() constructors now install
validation.SafeHTTPDialContext; webhook URLs (operator-
configured via dynamic-config GUI) cannot dial 169.254.x or
in-cluster reserved ranges. Test seam: newForTest bypasses
the guard for httptest's 127.0.0.1 binds, mirroring the
existing internal/connector/notifier/webhook pattern.
RT-L2 CERTCTL_ACME_INSECURE=true now emits a prominent
logger.Warn at server boot. Pre-Bundle-5 the knob silently
disabled ACME directory TLS verification.
Source findings closed (doc):
finding 1 + HIGH-5 Architecture doc claimed no in-process JWT/
OIDC/mTLS/SAML and pointed everyone at the
authenticating-gateway pattern. Auth Bundle 2
(commit dea5053) shipped native OIDC + sessions +
break-glass. New §"In-process authentication surface"
table (api-key / oidc / none) supersedes the old framing;
"Authenticating-gateway pattern (SAML, mTLS-as-auth,
LDAP)" section retained for protocols certctl still
doesn't ship natively.
Source findings verified false (existing implementation):
S4 OIDC email-domain allowlist — `email_domain_test.go`
already pins the strict-equality semantics (subdomain not
auto-accepted, multi-entry no-match path, empty allowlist
accepts all by-design per RFC 9700 §4.1.1).
SEC-L1 CSP / HSTS / referrer-policy headers — already shipped at
internal/api/middleware/securityheaders.go and wired at
cmd/server/main.go L2003+L2027+L2115.
Operator-decision / deferred (tracked in bundle-5 closure doc):
S3 CERTCTL_API_KEYS_NAMED parsing is wired, end-to-end
validation is partial. Operator decides: complete the
named-key middleware path or deprecate the syntax.
S5 Audit-middleware best-effort for read paths;
security-critical writes use WithinTx. Operator decides
per-path escalation.
S8 MCP threat model — the binary is a thin protocol bridge,
no privileges of its own; every tool call carries
CERTCTL_API_KEY and is auth'd + RBAC-gated server-side.
Optional CERTCTL_MCP_READ_ONLY gate tracked as v3.
SEC-H1 2026-05-10 audit CRIT-1/2/4 already closed on master;
CRIT-3/5 status against the spec folder is operator-
workstation-validation-only. Documented for follow-up.
SEC-L2 WebAuthn / FIDO2 / step-up — already documented in
docs/operator/auth-threat-model.md "Threats Bundle 2 does
NOT close". v3 work item per CLAUDE.md decision 12.
Full per-finding rationale + receipts at
docs/operator/security-bundle-5-audit-closure.md.
Verification:
gofmt -l # clean
go vet ./internal/connector/notifier/slack
./internal/connector/notifier/teams ./internal/auth/oidc
./internal/api/handler ./cmd/server # clean
go build ./cmd/server [...] # clean
go test -short -count=1 ./internal/connector/notifier/slack
./internal/connector/notifier/teams ./internal/api/handler
./internal/auth/oidc ./internal/config # PASS
# (slack 0.028s + teams
# 0.023s + handler 11.0s;
# newForTest seam keeps
# httptest tests green)
Audit-Closes: BUNDLE-5 S1 R6 R7 RT-L2 finding-1 HIGH-5
Audit-Verifies-False: S4 SEC-L1
Audit-Defers: S3 S5 S8 SEC-H1 SEC-L2
124 lines
3.7 KiB
Go
124 lines
3.7 KiB
Go
package slack
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
func TestSlack_Channel(t *testing.T) {
|
|
n := New(Config{WebhookURL: "https://hooks.slack.com/test"})
|
|
if n.Channel() != "Slack" {
|
|
t.Errorf("expected channel Slack, got %s", n.Channel())
|
|
}
|
|
}
|
|
|
|
func TestSlack_SendSuccess(t *testing.T) {
|
|
var receivedPayload slackMessage
|
|
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodPost {
|
|
t.Errorf("expected POST, got %s", r.Method)
|
|
}
|
|
if ct := r.Header.Get("Content-Type"); ct != "application/json" {
|
|
t.Errorf("expected application/json, got %s", ct)
|
|
}
|
|
if err := json.NewDecoder(r.Body).Decode(&receivedPayload); err != nil {
|
|
t.Fatalf("failed to decode payload: %v", err)
|
|
}
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
defer server.Close()
|
|
|
|
// Bundle 5 closure (R7): production `New` installs the SSRF dial-time
|
|
// guard which refuses httptest.NewServer's 127.0.0.1 bind. The
|
|
// unexported `newForTest` constructor bypasses the guard for unit
|
|
// tests that exercise the rest of the notifier path.
|
|
n := newForTest(Config{WebhookURL: server.URL})
|
|
err := n.Send(context.Background(), "ops@example.com", "Cert Expiring", "mc-api-prod expires in 7 days")
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
if !strings.Contains(receivedPayload.Text, "*Cert Expiring*") {
|
|
t.Errorf("expected bold subject in text, got %q", receivedPayload.Text)
|
|
}
|
|
if !strings.Contains(receivedPayload.Text, "mc-api-prod expires in 7 days") {
|
|
t.Errorf("expected body in text, got %q", receivedPayload.Text)
|
|
}
|
|
}
|
|
|
|
func TestSlack_SendWithOverrides(t *testing.T) {
|
|
var receivedPayload slackMessage
|
|
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
json.NewDecoder(r.Body).Decode(&receivedPayload)
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
defer server.Close()
|
|
|
|
n := newForTest(Config{
|
|
WebhookURL: server.URL,
|
|
ChannelOverride: "#alerts",
|
|
Username: "certctl-bot",
|
|
IconEmoji: ":lock:",
|
|
})
|
|
err := n.Send(context.Background(), "", "Test", "body")
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
if receivedPayload.Channel != "#alerts" {
|
|
t.Errorf("expected channel #alerts, got %s", receivedPayload.Channel)
|
|
}
|
|
if receivedPayload.Username != "certctl-bot" {
|
|
t.Errorf("expected username certctl-bot, got %s", receivedPayload.Username)
|
|
}
|
|
if receivedPayload.IconEmoji != ":lock:" {
|
|
t.Errorf("expected icon_emoji :lock:, got %s", receivedPayload.IconEmoji)
|
|
}
|
|
}
|
|
|
|
func TestSlack_SendHTTPError(t *testing.T) {
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusForbidden)
|
|
w.Write([]byte("invalid_token"))
|
|
}))
|
|
defer server.Close()
|
|
|
|
n := newForTest(Config{WebhookURL: server.URL})
|
|
err := n.Send(context.Background(), "", "Test", "body")
|
|
if err == nil {
|
|
t.Fatal("expected error, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "HTTP 403") {
|
|
t.Errorf("expected HTTP 403 in error, got %v", err)
|
|
}
|
|
}
|
|
|
|
func TestSlack_SendConnectionError(t *testing.T) {
|
|
n := newForTest(Config{WebhookURL: "http://127.0.0.1:1"})
|
|
err := n.Send(context.Background(), "", "Test", "body")
|
|
if err == nil {
|
|
t.Fatal("expected connection error, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "request failed") {
|
|
t.Errorf("expected 'request failed' in error, got %v", err)
|
|
}
|
|
}
|
|
|
|
func TestSlack_ClientHasTimeout(t *testing.T) {
|
|
n := New(Config{WebhookURL: "https://hooks.slack.com/test"})
|
|
if n.httpClient.Timeout == 0 {
|
|
t.Fatal("expected HTTP client timeout to be set, got 0")
|
|
}
|
|
expectedTimeout := 10 * time.Second
|
|
if n.httpClient.Timeout != expectedTimeout {
|
|
t.Errorf("expected timeout %v, got %v", expectedTimeout, n.httpClient.Timeout)
|
|
}
|
|
}
|