fix(hardening): close DoS, traversal, and crypto-correctness gaps

internal/api/decide.go

@@ -16,6 +16,11 @@ import (
 	"github.com/tzone85/themis/internal/policy"
 )
 
+// maxPolicyYAML bounds the policy document size accepted by /decide. 64 KiB
+// is far larger than any real Themis policy and keeps the YAML parser away
+// from pathological inputs.
+const maxPolicyYAML = 64 << 10 // 64 KiB
+
 // decideRequest is the JSON body shape POST /v1/tenants/{id}/decide accepts.
 type decideRequest struct {
 	AIChange     aichange.AIChange `json:"ai_change"`
@@ -47,6 +52,13 @@ func (s *server) handleDecide(w http.ResponseWriter, r *http.Request, id string)
 		writeError(w, http.StatusBadRequest, "policy_yaml is required")
 		return
 	}
+	// A Themis policy document is small; bound it independently of the
+	// overall body limit so an alias/anchor-bomb YAML can't burn CPU in
+	// the parser.
+	if len(req.PolicyYAML) > maxPolicyYAML {
+		writeError(w, http.StatusRequestEntityTooLarge, "policy_yaml too large")
+		return
+	}
 
 	bodies, err := decodeBodies(req.WorkdirFiles)
 	if err != nil {

internal/api/hardening_test.go

@@ -0,0 +1,77 @@
+package api
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/tzone85/themis/internal/aichange"
+)
+
+func TestAPI_SecurityHeadersPresent(t *testing.T) {
+	base, _, _ := seedTenantForDecide(t)
+	srv := httptest.NewServer(NewMux(base))
+	t.Cleanup(srv.Close)
+
+	resp, err := srv.Client().Get(srv.URL + "/v1/health")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+
+	want := map[string]string{
+		"X-Content-Type-Options": "nosniff",
+		"X-Frame-Options":        "DENY",
+		"Referrer-Policy":        "no-referrer",
+	}
+	for k, v := range want {
+		if got := resp.Header.Get(k); got != v {
+			t.Errorf("header %s = %q, want %q", k, got, v)
+		}
+	}
+}
+
+func TestAPI_Decide_PolicyYAMLTooLarge(t *testing.T) {
+	base, id, token := seedTenantForDecide(t)
+	srv := httptest.NewServer(NewMux(base))
+	t.Cleanup(srv.Close)
+
+	body := decideRequest{
+		AIChange: aichange.AIChange{
+			PRID: "gh:test#too-large", Actor: "claude_code",
+			TouchedFiles: []aichange.FileTouch{{Path: "README.md", ChangeKind: aichange.FileModified, BeforeHash: "a", AfterHash: "b"}},
+		},
+		PolicyYAML: strings.Repeat("#", maxPolicyYAML+1),
+	}
+	status, _ := postJSON(t, srv.Client(), srv.URL+"/v1/tenants/"+id+"/decide", token, body)
+	if status != http.StatusRequestEntityTooLarge {
+		t.Fatalf("status = %d, want 413", status)
+	}
+}
+
+func TestAPI_Decide_BodyTooLargeRejected(t *testing.T) {
+	base, id, token := seedTenantForDecide(t)
+	srv := httptest.NewServer(NewMux(base))
+	t.Cleanup(srv.Close)
+
+	// A body larger than maxRequestBody is truncated by MaxBytesReader,
+	// so the JSON decode fails and the handler returns 400 rather than
+	// buffering the whole payload into memory.
+	huge := bytes.Repeat([]byte("a"), maxRequestBody+1024)
+	req, err := http.NewRequest(http.MethodPost, srv.URL+"/v1/tenants/"+id+"/decide", bytes.NewReader(huge))
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("Authorization", "Bearer "+token)
+	resp, err := srv.Client().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Fatalf("status = %d, want 400", resp.StatusCode)
+	}
+}

internal/api/overrides.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"encoding/json"
-	"errors"
 	"net/http"
 	"strings"
 	"time"
@@ -154,7 +153,10 @@ func (s *server) handleOverrideClosePM(w http.ResponseWriter, r *http.Request, i
 		writeError(w, http.StatusInternalServerError, "append closed: "+err.Error())
 		return
 	}
-	refreshed, _ := s.readTenantEvents(id)
+	refreshed, err := s.readTenantEvents(id)
+	if err != nil {
+		writeError(w, http.StatusInternalServerError, err.Error())
+		return
+	}
 	writeJSON(w, http.StatusOK, override.Compute(refreshed, req.PRID, now))
-	_ = errors.New // keep imports tidy
 }

internal/api/server.go

@@ -17,10 +17,16 @@ import (
 // /v1/health endpoint can advertise something useful in production.
 var Version = "dev"
 
+// maxRequestBody caps the size of any request body the API will read. The
+// largest legitimate payload is a `decide` request carrying base64 workdir
+// files; 8 MiB is generous for that and bounds memory use per request.
+const maxRequestBody = 8 << 20 // 8 MiB
+
 // NewMux constructs the HTTP route table rooted at base (the Themis state
-// directory). Returns a *http.ServeMux that the caller wraps in their own
-// http.Server (decoupling listen/serve from routing).
-func NewMux(base string) *http.ServeMux {
+// directory). Returns an http.Handler that the caller wraps in their own
+// http.Server (decoupling listen/serve from routing). The handler is wrapped
+// with body-size limiting and baseline security headers.
+func NewMux(base string) http.Handler {
 	mux := http.NewServeMux()
 	srv := &server{base: base}
 
@@ -33,7 +39,21 @@ func NewMux(base string) *http.ServeMux {
 
 	// Embedded dashboard at /.
 	mux.HandleFunc("/", srv.handleDashboard)
-	return mux
+	return withMiddleware(mux)
+}
+
+// withMiddleware wraps the router with cross-cutting concerns: a hard cap on
+// request body size (memory-exhaustion defence) and baseline response headers
+// (MIME-sniffing, clickjacking, and referrer-leak defences).
+func withMiddleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
+		h := w.Header()
+		h.Set("X-Content-Type-Options", "nosniff")
+		h.Set("X-Frame-Options", "DENY")
+		h.Set("Referrer-Policy", "no-referrer")
+		next.ServeHTTP(w, r)
+	})
 }
 
 type server struct {

internal/auth/oidc.go

@@ -2,6 +2,8 @@ package auth
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -12,6 +14,11 @@ import (
 	"time"
 )
 
+// oidcRequestTimeout bounds a single userinfo call. The TokenStore interface
+// carries no context, so without this a stalled IdP would hold the handler
+// goroutine until the process dies.
+const oidcRequestTimeout = 10 * time.Second
+
 // OIDCTokenStore implements TokenStore against an OpenID Connect provider's
 // userinfo endpoint. Bearer tokens presented to RequireIdentity are
 // validated by calling the userinfo URL; the response's `tenant` and `role`
@@ -75,7 +82,9 @@ func (s *OIDCTokenStore) Lookup(presented string) (Identity, error) {
 		client = http.DefaultClient
 	}
 
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, s.IssuerUserinfoURL, nil)
+	ctx, cancel := context.WithTimeout(context.Background(), oidcRequestTimeout)
+	defer cancel()
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.IssuerUserinfoURL, nil)
 	if err != nil {
 		return Identity{}, fmt.Errorf("%w: build request: %v", ErrOIDC, err)
 	}
@@ -129,21 +138,29 @@ func DefaultClaimMapper(raw map[string]any) (Identity, error) {
 	return Identity{Tenant: tenant, Role: role, Description: description}, nil
 }
 
+// cacheKey derives the map key from a token digest so raw bearer tokens are
+// never held in memory beyond the in-flight request.
+func cacheKey(token string) string {
+	sum := sha256.Sum256([]byte(token))
+	return hex.EncodeToString(sum[:])
+}
+
 func (s *OIDCTokenStore) cacheGet(token string) (Identity, bool) {
 	if s.CacheTTL <= 0 {
 		return Identity{}, false
 	}
+	key := cacheKey(token)
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if s.cache == nil {
 		return Identity{}, false
 	}
-	c, ok := s.cache[token]
+	c, ok := s.cache[key]
 	if !ok {
 		return Identity{}, false
 	}
 	if time.Now().After(c.ExpiresAt) {
-		delete(s.cache, token)
+		delete(s.cache, key)
 		return Identity{}, false
 	}
 	return c.Identity, true
@@ -153,12 +170,13 @@ func (s *OIDCTokenStore) cachePut(token string, id Identity) {
 	if s.CacheTTL <= 0 {
 		return
 	}
+	key := cacheKey(token)
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if s.cache == nil {
 		s.cache = map[string]cachedIdentity{}
 	}
-	s.cache[token] = cachedIdentity{
+	s.cache[key] = cachedIdentity{
 		Identity:  id,
 		ExpiresAt: time.Now().Add(s.CacheTTL),
 	}

internal/cli/advise_cmd.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"path/filepath"
 
 	"github.com/spf13/cobra"
 
@@ -108,7 +107,3 @@ func resolveLLM(name string) advisor.LLM {
 	}
 	return advisor.NullLLM{}
 }
-
-// keep the linter quiet when the standalone filepath import is unused
-// during refactors.
-var _ = filepath.Join

internal/cli/ingest_cmd.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -128,11 +129,11 @@ func defaultIngestOutPath(base, id, prID string) string {
 	safe := strings.NewReplacer("/", "_", "#", "_", ":", "_", " ", "_").Replace(prID)
 	dir := tenantSubdir(base, id, "aichange")
 	_ = os.MkdirAll(dir, 0o700)
-	return dir + "/" + safe + ".json"
+	return filepath.Join(dir, safe+".json")
 }
 
 func tenantSubdir(base, id, name string) string {
-	return base + "/tenants/" + id + "/" + name
+	return filepath.Join(base, "tenants", id, name)
 }
 
 func emitIngestCompleted(base, id, adapter, prID, path string, fileCount int) error {

internal/cli/root.go

@@ -6,6 +6,8 @@ import (
 	"runtime"
 
 	"github.com/spf13/cobra"
+
+	"github.com/tzone85/themis/internal/tenant"
 )
 
 // Build metadata, injected at link time via ldflags. Defaults make a
@@ -36,6 +38,10 @@ func NewRootCmd() *cobra.Command {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		Version:       versionString(),
+		// Reject malformed tenant IDs before any subcommand builds a
+		// per-tenant filesystem path. Mirrors the HTTP route-boundary
+		// guard so a scripted `--id ../../etc` can't escape the state dir.
+		PersistentPreRunE: validateTenantIDFlag,
 	}
 	root.SetVersionTemplate("themis {{.Version}}\n")
 	root.AddCommand(newTenantCmd())
@@ -55,3 +61,19 @@ func NewRootCmd() *cobra.Command {
 	root.AddCommand(newAdviseCmd())
 	return root
 }
+
+// validateTenantIDFlag runs before every subcommand. When the command
+// exposes an `--id` flag and it is set, the value must be a well-formed
+// tenant identifier; otherwise path construction downstream could escape
+// the state directory.
+func validateTenantIDFlag(cmd *cobra.Command, _ []string) error {
+	f := cmd.Flags().Lookup("id")
+	if f == nil {
+		return nil
+	}
+	id := f.Value.String()
+	if id == "" {
+		return nil // required-ness is enforced separately via MarkFlagRequired
+	}
+	return tenant.ValidateID(id)
+}

internal/cli/serve_cmd.go

@@ -38,6 +38,12 @@ func runServe(cmd *cobra.Command, base, addr string) error {
 		Addr:              addr,
 		Handler:           mux,
 		ReadHeaderTimeout: 5 * time.Second,
+		// Bound the full request/response lifecycle so a slow client
+		// trickling a body (Slowloris) or refusing to read the response
+		// cannot pin a handler goroutine indefinitely.
+		ReadTimeout:  30 * time.Second,
+		WriteTimeout: 60 * time.Second,
+		IdleTimeout:  120 * time.Second,
 	}
 
 	ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM)

internal/cli/tenant_id_guard_test.go

@@ -0,0 +1,43 @@
+package cli
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+// TestRoot_RejectsTraversalTenantID asserts the PersistentPreRunE guard
+// blocks malformed tenant IDs before any subcommand builds a filesystem
+// path from them.
+func TestRoot_RejectsTraversalTenantID(t *testing.T) {
+	base := t.TempDir()
+	for _, id := range []string{"../../etc", "..", "Bad-Upper", "has space"} {
+		out := &bytes.Buffer{}
+		cmd := NewRootCmd()
+		cmd.SetOut(out)
+		cmd.SetErr(out)
+		cmd.SetArgs([]string{"ledger", "doctor", "--id", id, "--base", base})
+		err := cmd.Execute()
+		if err == nil {
+			t.Fatalf("id %q: expected error, got nil (out=%q)", id, out.String())
+		}
+		if !strings.Contains(err.Error(), "invalid tenant id") {
+			t.Fatalf("id %q: error = %v, want 'invalid tenant id'", id, err)
+		}
+	}
+}
+
+// TestRoot_AcceptsValidTenantID confirms the guard does not reject a
+// well-formed ID (the command itself may still fail later for other reasons,
+// but not with an invalid-id error).
+func TestRoot_AcceptsValidTenantID(t *testing.T) {
+	base := t.TempDir()
+	out := &bytes.Buffer{}
+	cmd := NewRootCmd()
+	cmd.SetOut(out)
+	cmd.SetErr(out)
+	cmd.SetArgs([]string{"ledger", "doctor", "--id", "acme-corp", "--base", base})
+	if err := cmd.Execute(); err != nil && strings.Contains(err.Error(), "invalid tenant id") {
+		t.Fatalf("valid id rejected: %v", err)
+	}
+}