From 36a8a7bcbbeed668db6f7b710758d504e27bf5bf Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sun, 31 May 2026 01:20:31 +0400 Subject: [PATCH 1/4] feat(caddy): make (hsts) snippet's Strict-Transport-Security value configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/schemas/gcp/gkeautopilotresource.json | 3 + pkg/clouds/k8s/types.go | 13 +++++ pkg/clouds/pulumi/kubernetes/caddy.go | 9 +++ .../kubernetes/caddy_hsts_value_test.go | 56 +++++++++++++++++++ .../pulumi/kubernetes/embed/caddy/Caddyfile | 9 ++- 5 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go diff --git a/docs/schemas/gcp/gkeautopilotresource.json b/docs/schemas/gcp/gkeautopilotresource.json index dcdfaad1..72594afc 100644 --- a/docs/schemas/gcp/gkeautopilotresource.json +++ b/docs/schemas/gcp/gkeautopilotresource.json @@ -49,6 +49,9 @@ "enable": { "type": "boolean" }, + "hstsValue": { + "type": "string" + }, "image": { "type": "string" }, diff --git a/pkg/clouds/k8s/types.go b/pkg/clouds/k8s/types.go index 5c03b189..b1649ee8 100644 --- a/pkg/clouds/k8s/types.go +++ b/pkg/clouds/k8s/types.go @@ -61,6 +61,19 @@ type CaddyConfig struct { // "Local" preserves the client source IP (required for correct X-Forwarded-For from Cloudflare). // "Cluster" (default) SNATs source IP to a node IP, losing the direct client IP. ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty" yaml:"externalTrafficPolicy,omitempty"` + // HSTSValue overrides the Strict-Transport-Security header value emitted + // by the embedded `(hsts)` snippet. When unset, the snippet emits the + // conservative default `max-age=31536000; includeSubDomains; preload` + // (byte-identical to the prior hard-coded value). Set to a different + // string to relax or tighten the policy without forking the snippet — + // e.g. omit `preload` to avoid committing the apex domain to the HSTS + // preload list, or shorten max-age during a staged rollout. + // + // Implementation: the snippet uses Caddy's `{$HSTS_VALUE:default}` env + // placeholder, evaluated at Caddy config parse time. When this field is + // non-nil, the value is set as the `HSTS_VALUE` env var on the Caddy + // container; otherwise the placeholder's default kicks in. + HSTSValue *string `json:"hstsValue,omitempty" yaml:"hstsValue,omitempty"` } type DisruptionBudget struct { diff --git a/pkg/clouds/pulumi/kubernetes/caddy.go b/pkg/clouds/pulumi/kubernetes/caddy.go index 32a5bc5c..862eb6a5 100644 --- a/pkg/clouds/pulumi/kubernetes/caddy.go +++ b/pkg/clouds/pulumi/kubernetes/caddy.go @@ -311,6 +311,15 @@ func DeployCaddyService(ctx *sdk.Context, caddy CaddyDeployment, input api.Resou TextVolumes: caddyVolumes, } + // HSTSValue overrides the Strict-Transport-Security value emitted by + // the embedded `(hsts)` snippet via Caddy's `{$HSTS_VALUE:default}` + // placeholder. When unset, the placeholder's default kicks in and the + // rendered Caddyfile is byte-identical to the prior version — no env + // var is added, no diff for existing consumers. + if v := lo.FromPtr(caddy.CaddyConfig).HSTSValue; v != nil { + deploymentConfig.StackConfig.Env["HSTS_VALUE"] = *v + } + // Prepare secret environment variables (e.g., GOOGLE_APPLICATION_CREDENTIALS for GCP) secretEnvs := make(map[string]string) if len(caddy.SecretEnvs) > 0 { diff --git a/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go b/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go new file mode 100644 index 00000000..38863643 --- /dev/null +++ b/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go @@ -0,0 +1,56 @@ +package kubernetes + +import ( + "testing" + + . "github.com/onsi/gomega" + "github.com/samber/lo" + + "github.com/simple-container-com/api/pkg/clouds/k8s" +) + +// TestCaddyfileEmbedHSTSPlaceholder locks in the runtime contract for +// CaddyConfig.HSTSValue: the `(hsts)` snippet in embed/caddy/Caddyfile +// MUST use Caddy's `{$HSTS_VALUE:default}` env-var placeholder, with the +// default preserving the prior literal value byte-for-byte. If a future +// refactor accidentally re-inlines the literal, this test fails and the +// HSTSValue field becomes silently inert. +func TestCaddyfileEmbedHSTSPlaceholder(t *testing.T) { + RegisterTestingT(t) + + content, err := Caddyconfig.ReadFile("embed/caddy/Caddyfile") + Expect(err).ToNot(HaveOccurred(), "embed Caddyfile must be readable") + + body := string(content) + + // The (hsts) snippet must contain the placeholder, not a hard-coded + // literal — that's the only mechanism by which CaddyConfig.HSTSValue + // can override the value at runtime. + Expect(body).To(ContainSubstring(`{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}`), + "(hsts) snippet must use Caddy's env-var placeholder so CaddyConfig.HSTSValue can override it") + + // The literal value (without the {$...} wrapper) must NOT appear as a + // bare token, otherwise we'd have two competing definitions. The + // previous hard-coded form was exactly the substring below — the test + // guards against it being reintroduced. + Expect(body).ToNot(MatchRegexp(`Strict-Transport-Security\s+"max-age=31536000;\s+includeSubDomains;\s+preload"\s*$`), + "the prior literal Strict-Transport-Security value must not be present alongside the placeholder") +} + +// TestCaddyConfig_HSTSValue_FieldShape locks the struct contract: the +// field must be `*string` (nilable, so unset means "use default"), with +// the documented JSON+YAML tag `hstsValue,omitempty`. A future rename +// to `HstsValue` or a tag drift to `hsts_value` would silently break +// every parent stack server.yaml that sets the new field. +func TestCaddyConfig_HSTSValue_FieldShape(t *testing.T) { + RegisterTestingT(t) + + cfg := k8s.CaddyConfig{} + // Nil-pointer assignment must be allowed (the unset case). + Expect(cfg.HSTSValue).To(BeNil()) + + // Setting a value must round-trip. + cfg.HSTSValue = lo.ToPtr("max-age=31536000; includeSubDomains") + Expect(cfg.HSTSValue).ToNot(BeNil()) + Expect(*cfg.HSTSValue).To(Equal("max-age=31536000; includeSubDomains")) +} diff --git a/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile b/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile index b32734ff..c9da7db2 100644 --- a/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile +++ b/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile @@ -3,8 +3,15 @@ } (hsts) { + # HSTS value uses Caddy's env-var-with-default placeholder so operators + # can override the snippet's policy from CaddyConfig.HSTSValue without + # forking the snippet. Default preserves the prior literal value byte + # for byte — consumers that don't set HSTSValue see no change. + # Caddy 2.x evaluates {$VAR:default} at config parse time; the default + # token extends to the closing `}` of the placeholder, so spaces and + # semicolons in the default string are safe. header { - Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" + Strict-Transport-Security "{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}" } @httpReq { header X-Forwarded-Proto http From 5be3d5277cbc18a12fc95e25026ff41d3fcac55f Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sun, 31 May 2026 01:38:52 +0400 Subject: [PATCH 2/4] review pass: empty-string guard, wiring test, trimmed comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/clouds/k8s/types.go | 16 +--- .../aws/cloudtrail_security_alerts_test.go | 1 + .../pulumi/docker/security_report_test.go | 8 +- pkg/clouds/pulumi/kubernetes/caddy.go | 21 +++-- .../kubernetes/caddy_hsts_value_test.go | 76 ++++++++++--------- .../pulumi/kubernetes/embed/caddy/Caddyfile | 8 +- pkg/util/json_test.go | 4 +- 7 files changed, 67 insertions(+), 67 deletions(-) diff --git a/pkg/clouds/k8s/types.go b/pkg/clouds/k8s/types.go index b1649ee8..055feab0 100644 --- a/pkg/clouds/k8s/types.go +++ b/pkg/clouds/k8s/types.go @@ -61,18 +61,10 @@ type CaddyConfig struct { // "Local" preserves the client source IP (required for correct X-Forwarded-For from Cloudflare). // "Cluster" (default) SNATs source IP to a node IP, losing the direct client IP. ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty" yaml:"externalTrafficPolicy,omitempty"` - // HSTSValue overrides the Strict-Transport-Security header value emitted - // by the embedded `(hsts)` snippet. When unset, the snippet emits the - // conservative default `max-age=31536000; includeSubDomains; preload` - // (byte-identical to the prior hard-coded value). Set to a different - // string to relax or tighten the policy without forking the snippet — - // e.g. omit `preload` to avoid committing the apex domain to the HSTS - // preload list, or shorten max-age during a staged rollout. - // - // Implementation: the snippet uses Caddy's `{$HSTS_VALUE:default}` env - // placeholder, evaluated at Caddy config parse time. When this field is - // non-nil, the value is set as the `HSTS_VALUE` env var on the Caddy - // container; otherwise the placeholder's default kicks in. + // HSTSValue overrides the Strict-Transport-Security value emitted by + // the embedded `(hsts)` snippet. Applies to every site this Caddy + // aggregator serves; there is no per-stack override. Unset or empty + // uses the snippet's default (`max-age=31536000; includeSubDomains; preload`). HSTSValue *string `json:"hstsValue,omitempty" yaml:"hstsValue,omitempty"` } diff --git a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go index 38328a3c..52321994 100644 --- a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go +++ b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go @@ -335,6 +335,7 @@ func TestApplyOverride_EmptyOverrideIsNoop(t *testing.T) { // - leading/trailing whitespace inside the braces (idiomatic indentation), // - internal parentheses from existing OR-groups, // - mixed AND/OR/&& at the top level. +// // If a future contributor writes a pattern with leading whitespace or extra braces, // the wrap output should still be syntactically valid CloudWatch. func TestApplyOverride_WorstCaseBasePattern(t *testing.T) { diff --git a/pkg/clouds/pulumi/docker/security_report_test.go b/pkg/clouds/pulumi/docker/security_report_test.go index b66f394f..4272a722 100644 --- a/pkg/clouds/pulumi/docker/security_report_test.go +++ b/pkg/clouds/pulumi/docker/security_report_test.go @@ -80,10 +80,10 @@ func TestStageSecurityReportScript_SanitizesResourceName(t *testing.T) { // Pulumi's ApplyT callback re-fires on every preview/up. The helper must // short-circuit when the deterministic path already exists — no rewrite, // no mtime churn beyond the explicit refresh. Asserts: -// 1. Second call returns identical path. -// 2. mtime was refreshed by the second call (so TTL sweep won't garbage- -// collect actively-referenced files between preview and apply). -// 3. File size unchanged (no rewrite happened — content was already correct). +// 1. Second call returns identical path. +// 2. mtime was refreshed by the second call (so TTL sweep won't garbage- +// collect actively-referenced files between preview and apply). +// 3. File size unchanged (no rewrite happened — content was already correct). func TestStageSecurityReportScript_StatShortCircuit(t *testing.T) { RegisterTestingT(t) diff --git a/pkg/clouds/pulumi/kubernetes/caddy.go b/pkg/clouds/pulumi/kubernetes/caddy.go index 862eb6a5..ab7cebb6 100644 --- a/pkg/clouds/pulumi/kubernetes/caddy.go +++ b/pkg/clouds/pulumi/kubernetes/caddy.go @@ -36,6 +36,18 @@ func isPulumiOutputSet(output sdk.StringOutput) bool { return output.ElementType() != nil } +// caddyHSTSEnv returns env-var additions overriding the `(hsts)` snippet's +// header. Empty string is treated as unset — Caddy's `os.LookupEnv` would +// otherwise treat a present-but-empty var as "found" and ship a broken +// `Strict-Transport-Security ""`. +func caddyHSTSEnv(cfg *k8s.CaddyConfig) map[string]string { + v := lo.FromPtr(cfg).HSTSValue + if v == nil || *v == "" { + return nil + } + return map[string]string{"HSTS_VALUE": *v} +} + func CaddyResource(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, params pApi.ProvisionParams) (*api.ResourceOutput, error) { if input.Descriptor.Type != k8s.ResourceTypeCaddy { return nil, errors.Errorf("unsupported caddy type %q", input.Descriptor.Type) @@ -311,13 +323,8 @@ func DeployCaddyService(ctx *sdk.Context, caddy CaddyDeployment, input api.Resou TextVolumes: caddyVolumes, } - // HSTSValue overrides the Strict-Transport-Security value emitted by - // the embedded `(hsts)` snippet via Caddy's `{$HSTS_VALUE:default}` - // placeholder. When unset, the placeholder's default kicks in and the - // rendered Caddyfile is byte-identical to the prior version — no env - // var is added, no diff for existing consumers. - if v := lo.FromPtr(caddy.CaddyConfig).HSTSValue; v != nil { - deploymentConfig.StackConfig.Env["HSTS_VALUE"] = *v + for k, v := range caddyHSTSEnv(caddy.CaddyConfig) { + deploymentConfig.StackConfig.Env[k] = v } // Prepare secret environment variables (e.g., GOOGLE_APPLICATION_CREDENTIALS for GCP) diff --git a/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go b/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go index 38863643..09b46aa9 100644 --- a/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go +++ b/pkg/clouds/pulumi/kubernetes/caddy_hsts_value_test.go @@ -1,6 +1,7 @@ package kubernetes import ( + "reflect" "testing" . "github.com/onsi/gomega" @@ -9,48 +10,53 @@ import ( "github.com/simple-container-com/api/pkg/clouds/k8s" ) -// TestCaddyfileEmbedHSTSPlaceholder locks in the runtime contract for -// CaddyConfig.HSTSValue: the `(hsts)` snippet in embed/caddy/Caddyfile -// MUST use Caddy's `{$HSTS_VALUE:default}` env-var placeholder, with the -// default preserving the prior literal value byte-for-byte. If a future -// refactor accidentally re-inlines the literal, this test fails and the -// HSTSValue field becomes silently inert. func TestCaddyfileEmbedHSTSPlaceholder(t *testing.T) { RegisterTestingT(t) - content, err := Caddyconfig.ReadFile("embed/caddy/Caddyfile") - Expect(err).ToNot(HaveOccurred(), "embed Caddyfile must be readable") - - body := string(content) - - // The (hsts) snippet must contain the placeholder, not a hard-coded - // literal — that's the only mechanism by which CaddyConfig.HSTSValue - // can override the value at runtime. - Expect(body).To(ContainSubstring(`{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}`), - "(hsts) snippet must use Caddy's env-var placeholder so CaddyConfig.HSTSValue can override it") - - // The literal value (without the {$...} wrapper) must NOT appear as a - // bare token, otherwise we'd have two competing definitions. The - // previous hard-coded form was exactly the substring below — the test - // guards against it being reintroduced. - Expect(body).ToNot(MatchRegexp(`Strict-Transport-Security\s+"max-age=31536000;\s+includeSubDomains;\s+preload"\s*$`), - "the prior literal Strict-Transport-Security value must not be present alongside the placeholder") + Expect(err).ToNot(HaveOccurred()) + Expect(string(content)).To(ContainSubstring(`{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}`)) } -// TestCaddyConfig_HSTSValue_FieldShape locks the struct contract: the -// field must be `*string` (nilable, so unset means "use default"), with -// the documented JSON+YAML tag `hstsValue,omitempty`. A future rename -// to `HstsValue` or a tag drift to `hsts_value` would silently break -// every parent stack server.yaml that sets the new field. func TestCaddyConfig_HSTSValue_FieldShape(t *testing.T) { RegisterTestingT(t) + ft, ok := reflect.TypeOf(k8s.CaddyConfig{}).FieldByName("HSTSValue") + Expect(ok).To(BeTrue()) + Expect(ft.Type.String()).To(Equal("*string")) + Expect(ft.Tag.Get("json")).To(Equal("hstsValue,omitempty")) + Expect(ft.Tag.Get("yaml")).To(Equal("hstsValue,omitempty")) +} - cfg := k8s.CaddyConfig{} - // Nil-pointer assignment must be allowed (the unset case). - Expect(cfg.HSTSValue).To(BeNil()) +func TestCaddyHSTSEnv_WiringContract(t *testing.T) { + RegisterTestingT(t) - // Setting a value must round-trip. - cfg.HSTSValue = lo.ToPtr("max-age=31536000; includeSubDomains") - Expect(cfg.HSTSValue).ToNot(BeNil()) - Expect(*cfg.HSTSValue).To(Equal("max-age=31536000; includeSubDomains")) + tests := []struct { + name string + cfg *k8s.CaddyConfig + want map[string]string + }{ + {name: "nil_config", cfg: nil, want: nil}, + {name: "unset_field", cfg: &k8s.CaddyConfig{}, want: nil}, + {name: "empty_string_treated_as_unset", cfg: &k8s.CaddyConfig{HSTSValue: lo.ToPtr("")}, want: nil}, + { + name: "explicit_value_overrides", + cfg: &k8s.CaddyConfig{HSTSValue: lo.ToPtr("max-age=31536000; includeSubDomains")}, + want: map[string]string{"HSTS_VALUE": "max-age=31536000; includeSubDomains"}, + }, + { + name: "max_age_zero_to_disable", + cfg: &k8s.CaddyConfig{HSTSValue: lo.ToPtr("max-age=0")}, + want: map[string]string{"HSTS_VALUE": "max-age=0"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := caddyHSTSEnv(tt.cfg) + if tt.want == nil { + Expect(got).To(BeEmpty()) + } else { + Expect(got).To(Equal(tt.want)) + } + }) + } } diff --git a/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile b/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile index c9da7db2..430b0eb7 100644 --- a/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile +++ b/pkg/clouds/pulumi/kubernetes/embed/caddy/Caddyfile @@ -3,13 +3,7 @@ } (hsts) { - # HSTS value uses Caddy's env-var-with-default placeholder so operators - # can override the snippet's policy from CaddyConfig.HSTSValue without - # forking the snippet. Default preserves the prior literal value byte - # for byte — consumers that don't set HSTSValue see no change. - # Caddy 2.x evaluates {$VAR:default} at config parse time; the default - # token extends to the closing `}` of the placeholder, so spaces and - # semicolons in the default string are safe. + # Default preserved byte-for-byte; override via CaddyConfig.HSTSValue. header { Strict-Transport-Security "{$HSTS_VALUE:max-age=31536000; includeSubDomains; preload}" } diff --git a/pkg/util/json_test.go b/pkg/util/json_test.go index 03d79150..8fea7757 100644 --- a/pkg/util/json_test.go +++ b/pkg/util/json_test.go @@ -7,8 +7,8 @@ import ( ) type sampleTarget struct { - Name string `json:"name"` - Count int `json:"count"` + Name string `json:"name"` + Count int `json:"count"` Tags []string `json:"tags"` } From b7b92fae8a49d8cee2648f31ce4bf658e3a8b4ba Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sun, 31 May 2026 01:42:13 +0400 Subject: [PATCH 3/4] chore(docker): trim verbose AddPreviousOutputInEnv comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/clouds/pulumi/docker/build_and_push.go | 32 ++++------------------ 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/pkg/clouds/pulumi/docker/build_and_push.go b/pkg/clouds/pulumi/docker/build_and_push.go index f71042c9..7c64b7a6 100644 --- a/pkg/clouds/pulumi/docker/build_and_push.go +++ b/pkg/clouds/pulumi/docker/build_and_push.go @@ -253,33 +253,11 @@ func executeSecurityOperations(ctx *sdk.Context, stack api.Stack, dockerImage *d return fmt.Sprintf("sh %s", shellQuote(path)) }).(sdk.StringOutput), - // Disable the default behaviour of passing the previous run's - // stdout/stderr back as PULUMI_COMMAND_STDOUT / PULUMI_COMMAND_STDERR - // env vars on Update. pulumi-command's - // `run()` builds envp with: - // - // if in.AddPreviousOutputInEnv == nil || *in.AddPreviousOutputInEnv { - // if out.Stdout != "" { - // cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", - // util.PulumiCommandStdout, out.Stdout)) - // } - // ... - // } - // - // The security-report script writes the full rendered vulnerability - // table to stdout for inline build-log visibility. On a chrome-base- - // derived image that's a 150-200 KB markdown table (thousands of CVE - // rows). pulumi-command captures that into the resource's `Stdout` - // output, which is then echoed back as a SINGLE envp entry on the - // next Update. Kernel ARG_MAX (~128 KB on Linux) covers argv+envp - // combined, so the next deploy fails at `fork/exec /bin/sh: argument - // list too long` — even though our argv is now ~80 bytes thanks to - // tempfile staging. - // - // Our security scripts don't read PULUMI_COMMAND_STDOUT; turning - // this off is purely a no-op for the script's behaviour. The - // stdout is still captured into state for `pulumi stack output` and - // downstream resource references, just not re-injected into envp. + // Suppress pulumi-command's default Stdout-into-envp re-injection on + // Update: the security-report script writes a ~150-200 KB CVE table + // to stdout, which combined with argv exhausts kernel ARG_MAX. Our + // scripts don't read PULUMI_COMMAND_STDOUT, so this is a no-op for + // behaviour; stdout still lands in state for downstream refs. AddPreviousOutputInEnv: sdk.Bool(false), }, sdk.DependsOn(reportDeps)) if err != nil { From 787aeeeeff853ffa0657140e97db199c6e9db23a Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sun, 31 May 2026 02:48:43 +0400 Subject: [PATCH 4/4] chore(security): suppress 2 known-FP vulns in osv-scanner.toml + drop gofmt drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- osv-scanner.toml | 21 +++++++++++++++++++ .../aws/cloudtrail_security_alerts_test.go | 1 - .../pulumi/docker/security_report_test.go | 8 +++---- pkg/util/json_test.go | 4 ++-- 4 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 osv-scanner.toml diff --git a/osv-scanner.toml b/osv-scanner.toml new file mode 100644 index 00000000..98172750 --- /dev/null +++ b/osv-scanner.toml @@ -0,0 +1,21 @@ +# OSV-Scanner ignore list. Mirrors the `not_affected` statements in +# `vex/openvex.json` for the benefit of OpenSSF Scorecard's Vulnerabilities +# check, which queries osv.dev directly and doesn't read VEX. The VEX file +# remains the source-of-truth justification; this file is a re-statement +# in a format the scanner understands. +# +# Add an entry here ONLY after the corresponding `not_affected` statement +# lands in vex/openvex.json (review by code owners under the default +# `*` CODEOWNERS rule). Suppressing a real vulnerability here without VEX +# is a regression — track upstream-fix availability in the VEX +# impact_statement, and remove from BOTH files when fixable. +# +# Reference: https://google.github.io/osv-scanner/configuration/ + +[[IgnoredVulns]] +id = "GO-2022-0635" +reason = "VEX not_affected: github.com/aws/aws-sdk-go is required transitively by github.com/pulumi/pulumi/pkg/v3/operations; the s3crypto subpackage is never imported by this codebase nor any reachable code path. govulncheck -mode=source confirms 0 reachable vulnerabilities. Full justification in vex/openvex.json." + +[[IgnoredVulns]] +id = "GO-2022-0646" +reason = "VEX not_affected: same s3crypto unreachability as GO-2022-0635 — different advisory ID against the same subpackage. govulncheck confirms zero reachable calls to the affected symbols. Full justification in vex/openvex.json." diff --git a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go index 52321994..38328a3c 100644 --- a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go +++ b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go @@ -335,7 +335,6 @@ func TestApplyOverride_EmptyOverrideIsNoop(t *testing.T) { // - leading/trailing whitespace inside the braces (idiomatic indentation), // - internal parentheses from existing OR-groups, // - mixed AND/OR/&& at the top level. -// // If a future contributor writes a pattern with leading whitespace or extra braces, // the wrap output should still be syntactically valid CloudWatch. func TestApplyOverride_WorstCaseBasePattern(t *testing.T) { diff --git a/pkg/clouds/pulumi/docker/security_report_test.go b/pkg/clouds/pulumi/docker/security_report_test.go index 4272a722..b66f394f 100644 --- a/pkg/clouds/pulumi/docker/security_report_test.go +++ b/pkg/clouds/pulumi/docker/security_report_test.go @@ -80,10 +80,10 @@ func TestStageSecurityReportScript_SanitizesResourceName(t *testing.T) { // Pulumi's ApplyT callback re-fires on every preview/up. The helper must // short-circuit when the deterministic path already exists — no rewrite, // no mtime churn beyond the explicit refresh. Asserts: -// 1. Second call returns identical path. -// 2. mtime was refreshed by the second call (so TTL sweep won't garbage- -// collect actively-referenced files between preview and apply). -// 3. File size unchanged (no rewrite happened — content was already correct). +// 1. Second call returns identical path. +// 2. mtime was refreshed by the second call (so TTL sweep won't garbage- +// collect actively-referenced files between preview and apply). +// 3. File size unchanged (no rewrite happened — content was already correct). func TestStageSecurityReportScript_StatShortCircuit(t *testing.T) { RegisterTestingT(t) diff --git a/pkg/util/json_test.go b/pkg/util/json_test.go index 8fea7757..03d79150 100644 --- a/pkg/util/json_test.go +++ b/pkg/util/json_test.go @@ -7,8 +7,8 @@ import ( ) type sampleTarget struct { - Name string `json:"name"` - Count int `json:"count"` + Name string `json:"name"` + Count int `json:"count"` Tags []string `json:"tags"` }