Skip to content

feat(caddy): make (hsts) snippet's Strict-Transport-Security value configurable#306

Merged
Cre-eD merged 4 commits into
mainfrom
feat/caddy-hsts-value-configurable
Jun 1, 2026
Merged

feat(caddy): make (hsts) snippet's Strict-Transport-Security value configurable#306
Cre-eD merged 4 commits into
mainfrom
feat/caddy-hsts-value-configurable

Conversation

@Cre-eD

@Cre-eD Cre-eD commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds CaddyConfig.HSTSValue *string so operators can override the Strict-Transport-Security value emitted by the embedded (hsts) snippet without forking the snippet definition. When unset, the snippet defaults to the prior literal max-age=31536000; includeSubDomains; preload byte-for-byte — no diff for existing consumers.

Motivation

The snippet's default ships the preload directive. Including preload is a sticky one-way commitment: once a domain is submitted to the HSTS preload list (the static list browsers ship with), removal requires asking the browser vendor and waiting months. Operators may want to relax to max-age=31536000; includeSubDomains (no preload) for the rollout phase, or further during a staged migration, without losing the snippet's other behaviors (HTTP→HTTPS redirect on X-Forwarded-Proto: http).

Implementation

  • embed/caddy/Caddyfile(hsts) snippet now uses Caddy's native env-var placeholder:

    Strict-Transport-Security "{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}"
    

    Caddy resolves {$VAR:default} at config parse time; the default token extends to the matching }, so spaces and semicolons in the default string survive the parser intact.

  • pkg/clouds/k8s/types.go — adds HSTSValue *string to CaddyConfig, documented inline (yaml tag hstsValue,omitempty).

  • pkg/clouds/pulumi/kubernetes/caddy.go — when CaddyConfig.HSTSValue != nil, sets the HSTS_VALUE env var on the Caddy container via deploymentConfig.StackConfig.Env. When nil, no env var is added and the Caddyfile placeholder's default kicks in.

  • docs/schemas/gcp/gkeautopilotresource.json — schema gains hstsValue: { type: string } so the field is discoverable in tooling.

Tests

  • TestCaddyfileEmbedHSTSPlaceholder — asserts the snippet uses the placeholder form (not the prior bare literal). Guards against a refactor silently re-inlining the literal and making HSTSValue inert.
  • TestCaddyConfig_HSTSValue_FieldShape — locks the struct contract: *string (nilable so unset = use default), yaml tag hstsValue,omitempty.

Compatibility

  • Unset (existing consumers): byte-identical Caddyfile content + no env var added → zero behavioral diff.
  • Set: env var lands on the Caddy container; placeholder substitutes at Caddy parse time. The previous deploy's Caddyfile / annotation state is preserved exactly, so no spurious Pulumi diff on stacks that don't opt in.

Caveats / scope

  • The env var lives on the parent Caddy aggregator pod, not the per-stack consumer. The value is set once per cluster (via the parent stack's caddy.hstsValue); all sites the aggregator serves get the same HSTS value. Per-site override would require a different mechanism (e.g., per-stack lbConfig.siteExtraHelpers + header_down) and is out of scope here.

Test plan

  • go build ./pkg/... + go vet ./pkg/... pass.
  • Both regression tests pass.
  • CI: full pkg/clouds/pulumi/kubernetes/... suite green.
  • Manual: SC preview build → deploy a stack with caddy.hstsValue: "max-age=86400"curl -sIL <host>/ | grep -i strict-transport-security returns the overridden value, not the default.

@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown

Semgrep Scan Results

Repository: api | Commit: 9ba8f0a

Check Status Details
✅ Semgrep Pass 0 total findings (no error/warning)

Scanned at 2026-05-30 22:49 UTC

@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown

Security Scan Results

Repository: api | Commit: 9ba8f0a

Check Status Details
✅ Secret Scan Pass No secrets detected
✅ Dependencies (Trivy) Pass 0 total (no critical/high)
✅ Dependencies (Grype) Pass 0 total (no critical/high)
📦 SBOM Generated 527 components (CycloneDX)

Scanned at 2026-05-30 22:49 UTC

Cre-eD added 3 commits May 31, 2026 01:40
…nfigurable

Adds `CaddyConfig.HSTSValue *string` so operators can override the
header value emitted by the embedded `(hsts)` snippet without forking
the snippet definition. When unset, the snippet defaults to the prior
literal value (`max-age=31536000; includeSubDomains; preload`) byte-for-
byte — no diff for existing consumers.

Why this is useful: the snippet's default ships the `preload` directive.
Including `preload` is a sticky commitment — once a domain is submitted
to the HSTS preload list (chromium.googlesource.com/chromium/src/+/HEAD/
net/http/transport_security_state_static.json), it ships hard-coded in
browsers and removal takes months. Operators may want to relax to
`max-age=31536000; includeSubDomains` (no preload) for the rollout
phase, or further while a staged migration is in flight, without losing
access to the snippet's other behaviors (HTTP→HTTPS redirect on
`X-Forwarded-Proto: http`).

Implementation:
- `embed/caddy/Caddyfile` (hsts) snippet now uses Caddy's native env-var
  placeholder `{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}`.
  Caddy resolves the placeholder at config parse time; the default token
  extends to the matching `}`, so spaces and semicolons in the default
  string survive the parser intact.
- `pkg/clouds/k8s/types.go` adds `HSTSValue *string` to `CaddyConfig`,
  documented inline.
- `pkg/clouds/pulumi/kubernetes/caddy.go` sets the `HSTS_VALUE` env var
  on the Caddy container when `CaddyConfig.HSTSValue != nil`. When nil,
  the env var is not added, and the Caddyfile placeholder's default
  kicks in — byte-identical rendered Caddyfile for existing consumers.
- `docs/schemas/gcp/gkeautopilotresource.json` schema gains the new
  field so it's discoverable in tooling.

Tests:
- TestCaddyfileEmbedHSTSPlaceholder asserts the snippet uses the
  placeholder form (not the prior bare literal). Guards against a
  refactor silently re-inlining the literal and making HSTSValue inert.
- TestCaddyConfig_HSTSValue_FieldShape locks the struct contract:
  *string (nilable so unset = use default), yaml tag `hstsValue,omitempty`.

Compatibility:
- Unset: byte-identical Caddyfile + no env var → existing deploys see
  zero change.
- Set: env var lands on Caddy container; placeholder substitutes at
  Caddy parse time. The previous deploy's annotation/Caddyfile state is
  preserved exactly, so no spurious Pulumi diff on stacks that don't
  opt in.

Caveat: the env var lives on the parent Caddy aggregator pod, not the
per-stack consumer. The value is set once per cluster (via the parent
stack's `caddy.hstsValue`); all sites the aggregator serves get the
same HSTS value. Per-site override would require a different mechanism
(e.g., per-stack `lbConfig.siteExtraHelpers` + `header_down`) and is
out of scope here.

Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Addresses 3-reviewer feedback on the original revision (claude-opus-4-8
+ codex + gemini converged on these):

- Empty-string footgun: `HSTSValue = lo.ToPtr("")` would set the env
  var to "" and Caddy's `os.LookupEnv` treats present-but-empty as
  "found", skipping the placeholder default → ships
  `Strict-Transport-Security ""`. Guard: empty is treated as unset.
- Extract `caddyHSTSEnv()` as a pure helper so the env-var wiring is
  testable without spinning up Pulumi. Previously a one-liner inside
  a 300-line deploy function — a regression deleting it would have
  silenced HSTSValue without any test failing.
- Tighten the field-shape test to assert struct tags via reflection
  (the prior assertion only set/dereffed the value, would not have
  caught a `yaml:"hsts_value"` tag drift).
- Drop the brittle negative regex in the embed-content test; the
  positive substring assertion is sufficient.
- Trim verbose comments throughout (Caddyfile, types.go, caddy.go).

Empty case is now covered by TestCaddyHSTSEnv_WiringContract; all 7
relevant tests pass; build + vet clean.

Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
The 28-line block on a 1-line directive — the embedded pulumi-command
source snippet + multi-paragraph context — was disproportionate. Reduced
to a 5-line summary that captures WHY (ARG_MAX from re-injected stdout)
and the safety claim (scripts don't read PULUMI_COMMAND_STDOUT). Full
context lives in PR #305's commit message + body for future readers.

Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
@Cre-eD
Cre-eD force-pushed the feat/caddy-hsts-value-configurable branch from 56f1f04 to b7b92fa Compare May 30, 2026 21:42
@Cre-eD

Cre-eD commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

End-to-end behavior verified

Built preview v2026.5.44-pre.b7b92fa-preview.b7b92fa. Ran a local Caddy 2.11.3 instance with the exact (hsts) snippet syntax from this PR's embed/caddy/Caddyfile:

(hsts) {
  header {
     Strict-Transport-Security "{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}"
  }
}

:8080 {
  respond "ok"
  import hsts
}

Three scenarios via docker run … caddy:2.11.3 with different env settings:

HSTS_VALUE env Response header value
unset max-age=31536000; includeSubDomains; preload (placeholder default)
"max-age=86400; includeSubDomains" max-age=86400; includeSubDomains
"" (empty string) `` (empty — confirms the reviewers' footgun)

The empty-string case is exactly the silent-broken-header scenario the v2 review pass guarded against in caddyHSTSEnv() (v != nil && *v != ""); without that guard, a parent stack setting caddy.hstsValue: "" would ship an empty header. The Go-side helper is also unit-tested across all 5 cases (TestCaddyHSTSEnv_WiringContract).

This is the canonical syntax — Caddy 2.x parses the placeholder, the default token extends across spaces and semicolons unchanged, and the empty-string trap is closed at the SC api layer (not Caddy's).

Ready to merge.

… gofmt drift

Adds osv-scanner.toml at repo root listing the two not_affected GO-* IDs
that vex/openvex.json already documents. OpenSSF Scorecard's
Vulnerabilities check runs osv-scanner directly against osv.dev — it
doesn't read VEX — so the same two advisories trip the Scorecard build
on every run despite the VEX statements.

Per the OpenSSF guidance
(https://github.com/ossf/scorecard/blob/main/docs/checks.md#vulnerabilities):

  > If you believe the vulnerability does not affect your project, the
  > vulnerability can be ignored. To ignore, create an osv-scanner.toml
  > file next to the dependency manifest…

VEX remains the source-of-truth justification; this file is a
re-statement in a format osv-scanner understands. The `reason` field
on each entry refers back to vex/openvex.json + a one-line summary,
not a fresh justification — a regression in either file should still
fail review.

IgnoredVulns added:
- GO-2022-0635 — aws-sdk-go/service/s3/s3crypto unreachable (transitively
  pulled via pulumi/pulumi/pkg/v3/operations; govulncheck confirms 0
  reachable calls).
- GO-2022-0646 — same s3crypto unreachability against a different
  advisory ID.

Also drops gofmt drift on three unrelated test files
(cloudtrail_security_alerts_test.go, security_report_test.go,
json_test.go) that got swept in by an earlier `gofmt -w pkg/` on this
branch — they're pre-existing unformatted state in main that should be
addressed in a dedicated cleanup PR, not bundled into a feature PR.

Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
@Cre-eD
Cre-eD merged commit 53f8370 into main Jun 1, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants