diff --git a/pkg/clouds/pulumi/kubernetes/simple_container.go b/pkg/clouds/pulumi/kubernetes/simple_container.go index d16b41d9..1535e952 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "regexp" + "sort" "strconv" "strings" @@ -706,16 +707,37 @@ ${proto}://${domain} { if args.UseSSL { imports = append(imports, "import hsts") } + // Render `args.Headers` as one `header_down` directive per line and + // quote the value so multi-token headers (CSP, Permissions-Policy) + // survive Caddy's whitespace tokenizer. The leading "\n " prevents + // the first entry from being concatenated onto the + // `header_down Server nginx ${addHeaders}` line in the template, + // which would produce an invalid Caddyfile. Entries are sorted by key + // for deterministic output (Go map iteration order is randomized). + // + // `%q` round-trips through Caddy's quoted-string lexer correctly for + // embedded `"` (→ `\"` → `"`) and `\` (→ `\\` → `\`). HTTP header + // values MUST be single-line ASCII (RFC 9110), so the Go-vs-Caddy + // divergence on `\n`/`\t`/`\xNN` is outside the legal-input domain; + // pathological inputs round-trip as a literal `\n` byte sequence + // instead of a newline, which is the safer failure mode. + addHeadersStr := "" + if hdrs := lo.FromPtr(args.Headers); len(hdrs) > 0 { + keys := lo.Keys(hdrs) + sort.Strings(keys) + lines := lo.Map(keys, func(k string, _ int) string { + return fmt.Sprintf("header_down %s %q", k, hdrs[k]) + }) + addHeadersStr = "\n " + strings.Join(lines, "\n ") + } placeholdersMap := placeholders.MapData{ - "proto": lo.If(lo.FromPtr(args.LbConfig).Https, "https").Else("http"), - "domain": args.Domain, - "prefix": args.Prefix, - "service": sanitizedService, - "namespace": sanitizedNamespace, - "port": strconv.Itoa(lo.FromPtr(mainPort)), - "addHeaders": strings.Join(lo.Map(lo.Entries(lo.FromPtr(args.Headers)), func(h lo.Entry[string, string], _ int) string { - return fmt.Sprintf("header_down %s %s", h.Key, h.Value) - }), "\n "), + "proto": lo.If(lo.FromPtr(args.LbConfig).Https, "https").Else("http"), + "domain": args.Domain, + "prefix": args.Prefix, + "service": sanitizedService, + "namespace": sanitizedNamespace, + "port": strconv.Itoa(lo.FromPtr(mainPort)), + "addHeaders": addHeadersStr, "extraHelpers": strings.Join(lo.FromPtr(args.LbConfig).ExtraHelpers, "\n "), "imports": strings.Join(imports, "\n "), } diff --git a/pkg/clouds/pulumi/kubernetes/simple_container_test.go b/pkg/clouds/pulumi/kubernetes/simple_container_test.go index 095ada2a..c5255090 100644 --- a/pkg/clouds/pulumi/kubernetes/simple_container_test.go +++ b/pkg/clouds/pulumi/kubernetes/simple_container_test.go @@ -1,6 +1,8 @@ package kubernetes import ( + "regexp" + "strings" "testing" . "github.com/onsi/gomega" @@ -424,6 +426,197 @@ func TestNewSimpleContainer_ComplexConfiguration(t *testing.T) { Expect(err).ToNot(HaveOccurred(), "Test should complete without errors") } +// CaddyfileEntry Header Rendering Tests +// +// Regression coverage for two coupled bugs in the `args.Headers` rendering +// path: (a) the first `header_down` was being concatenated onto the +// `header_down Server nginx ${addHeaders}` template line, producing +// invalid Caddyfile syntax, and (b) values weren't quoted, so multi-token +// headers (CSP, Permissions-Policy) broke Caddy's whitespace tokenizer. +// Both stay fixed only if these assertions hold. + +// caddyfileEntryFor returns the rendered Caddyfile entry for the given +// args. Uses the same pulumi-mocks pattern as the rest of this file. +func caddyfileEntryFor(t *testing.T, args *SimpleContainerArgs) string { + t.Helper() + mocks := NewSimpleContainerMocks() + var rendered string + err := pulumi.RunErr(func(ctx *pulumi.Context) error { + sc, err := NewSimpleContainer(ctx, args) + if err != nil { + return err + } + rendered = string(sc.CaddyfileEntry) + return nil + }, pulumi.WithMocks("project", "stack", mocks)) + Expect(err).ToNot(HaveOccurred(), "pulumi run should not fail") + return rendered +} + +func TestCaddyfileEntry_HeadersRenderOnTheirOwnLines(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.Headers = &k8s.Headers{ + "X-Frame-Options": "DENY", + "X-Content-Type-Options": "nosniff", + "Referrer-Policy": "strict-origin-when-cross-origin", + } + + entry := caddyfileEntryFor(t, args) + + // The bug: first custom header_down landed on the same line as + // `header_down Server nginx`. Assert each of our headers gets its + // own line. + for _, hdr := range []string{"X-Frame-Options", "X-Content-Type-Options", "Referrer-Policy"} { + // `(?m)^[\t ]*header_down ` ensures the directive starts a line + // (only leading whitespace allowed before it). + Expect(entry).To(MatchRegexp(`(?m)^[\t ]*header_down `+regexp.QuoteMeta(hdr)+` `), + "header_down for %s must start its own line, got:\n%s", hdr, entry) + } + + // And the `Server nginx` directive must end its own line — i.e. the + // next non-whitespace token after `nginx` is a newline. + Expect(entry).To(MatchRegexp(`header_down Server nginx[\t ]*\n`), + "`header_down Server nginx` must be followed by a newline, got:\n%s", entry) +} + +func TestCaddyfileEntry_MultiTokenHeaderValuesAreQuoted(t *testing.T) { + RegisterTestingT(t) + + csp := `default-src 'self'; script-src 'self' 'unsafe-inline' https://example.com; frame-ancestors 'none'` + permissions := `geolocation=(), camera=(), microphone=(), payment=()` + + args := createBasicTestArgs() + args.Headers = &k8s.Headers{ + "Content-Security-Policy-Report-Only": csp, + "Permissions-Policy": permissions, + } + + entry := caddyfileEntryFor(t, args) + + // Caddy tokenizes on whitespace; multi-word header values MUST be + // wrapped in double quotes or the parser sees them as extra args. + // %q also escapes embedded double quotes — there are none in CSP, but + // the literal single quotes around 'self' are passed through. + Expect(entry).To(ContainSubstring(`header_down Content-Security-Policy-Report-Only "`+csp+`"`), + "CSP value must be double-quoted in the rendered Caddyfile, got:\n%s", entry) + Expect(entry).To(ContainSubstring(`header_down Permissions-Policy "`+permissions+`"`), + "Permissions-Policy value must be double-quoted, got:\n%s", entry) +} + +func TestCaddyfileEntry_HeadersAreSortedDeterministically(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + args.Headers = &k8s.Headers{ + "X-Frame-Options": "DENY", + "A-Header": "first-alphabetically", + "Referrer-Policy": "no-referrer", + "X-Content-Type-Options": "nosniff", + } + + // Two-render equality is probabilistic with only 4 keys (~75% miss + // rate even with broken sorting), so it's a weak signal on its own. + // The load-bearing assertion is the alphabetical index check below. + a := caddyfileEntryFor(t, args) + b := caddyfileEntryFor(t, args) + Expect(a).To(Equal(b), "two renders with the same Headers map must be byte-identical") + + // This is what actually proves determinism: deterministic alphabetical + // ordering (A-Header before Referrer-Policy before X-* etc.) so we + // can rely on the rendered bytes for the change-hash signal. + idxA := strings.Index(a, "header_down A-Header ") + idxR := strings.Index(a, "header_down Referrer-Policy ") + idxXC := strings.Index(a, "header_down X-Content-Type-Options ") + idxXF := strings.Index(a, "header_down X-Frame-Options ") + Expect(idxA).To(BeNumerically(">", 0), "A-Header must be present") + Expect(idxA).To(BeNumerically("<", idxR), "A-Header before Referrer-Policy") + Expect(idxR).To(BeNumerically("<", idxXC), "Referrer-Policy before X-Content-Type-Options") + Expect(idxXC).To(BeNumerically("<", idxXF), "X-Content-Type-Options before X-Frame-Options") +} + +func TestCaddyfileEntry_EmptyHeadersStillRenders(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + // args.Headers is &k8s.Headers{} (empty) from createBasicTestArgs. + entry := caddyfileEntryFor(t, args) + + // Server nginx still rendered, no spurious `header_down` directives, + // no trailing garbage from a leaked newline. + Expect(entry).To(ContainSubstring("header_down Server nginx")) + Expect(entry).ToNot(MatchRegexp(`header_down Server nginx[^\n]+header_down`), + "with empty Headers, the Server nginx line must not be followed by any other header_down on the same line, got:\n%s", entry) +} + +func TestCaddyfileEntry_EmptyHeadersByteIdenticalToPreFix(t *testing.T) { + RegisterTestingT(t) + + args := createBasicTestArgs() + // args.Headers is &k8s.Headers{} (empty) from createBasicTestArgs. + entry := caddyfileEntryFor(t, args) + + // Lock in the compatibility contract: when Headers is empty, the + // rendered Caddyfile must contain `header_down Server nginx ` followed + // immediately by a newline (the trailing space comes from the literal + // space in the template between `nginx` and the now-empty `${addHeaders}` + // substitution). Pre-fix output had exactly this shape; we must not + // drift it, or the parent Caddy aggregator sees a spurious change-hash + // flap on every existing header-less stack after the SC upgrade. + Expect(entry).To(ContainSubstring("header_down Server nginx \n"), + "empty-Headers output must end the Server-nginx line with a single space + newline (byte-identical to pre-fix), got:\n%s", entry) +} + +func TestCaddyfileEntry_HeaderValueWithEmbeddedDoubleQuote(t *testing.T) { + RegisterTestingT(t) + + // CSP with a strict-dynamic attribute that uses double-quoted hashes + // is a realistic case that contains a literal `"`. `%q` must escape it + // so Caddy's lexer round-trips back to the original value. + value := `default-src 'self'; script-src 'self' "sha256-abc=" "strict-dynamic"` + + args := createBasicTestArgs() + args.Headers = &k8s.Headers{ + "Content-Security-Policy": value, + } + + entry := caddyfileEntryFor(t, args) + + // %q emits each embedded `"` as `\"`. Caddy's quoted-string lexer + // supports `\"` and `\\` as escape sequences; on round-trip the header + // value the server sets equals `value` exactly. This assertion locks in + // the escape behaviour — without it, a future refactor to bare `%s` + // would silently break any header value that contains a quote. + escaped := strings.ReplaceAll(value, `"`, `\"`) + Expect(entry).To(ContainSubstring(`header_down Content-Security-Policy "`+escaped+`"`), + "value containing embedded \" must be %%q-escaped (\\\") in the rendered Caddyfile, got:\n%s", entry) +} + +func TestCaddyfileEntry_HeadersOnPrefixTemplate(t *testing.T) { + RegisterTestingT(t) + + // The Prefix template branch (handle_path /* ...) shares the + // same addHeaders placeholder as the Domain branch but at a different + // indent level. Without a test here, the second template variant has + // zero CI coverage of the header-rendering path. + args := createBasicTestArgs() + args.Domain = "" + args.Prefix = "api" + args.Headers = &k8s.Headers{ + "X-Frame-Options": "DENY", + "Permissions-Policy": "geolocation=(), camera=()", + } + + entry := caddyfileEntryFor(t, args) + + // Both directives must appear on their own lines and the + // multi-token Permissions-Policy must be quoted, same contract as + // the Domain template. + Expect(entry).To(MatchRegexp(`(?m)^[\t ]*header_down X-Frame-Options `)) + Expect(entry).To(ContainSubstring(`header_down Permissions-Policy "geolocation=(), camera=()"`)) +} + // Name Sanitization Tests func TestNewSimpleContainer_NameSanitization(t *testing.T) {