From b886a755327ea0aa78db65d8be959f6b1796cc0e Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Wed, 9 Sep 2026 08:51:38 -0500 Subject: [PATCH] deploy: make appview billing opt-in, and verify what was built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build-trixie hardcoded `-tags billing`, so the only way to build a non-billing appview was to bypass the Makefile and drive the trixie container by hand. Both production deploys of this deployment did exactly that, and the workspace notes record it as "the deploy tool cannot reproduce this build" — a tool that cannot produce the artifact you actually ship is a tool nobody can safely use. Worse, the failure is silent and one-directional. Nothing about a billing binary looks different: same name, same version stamp, same vcs.revision. A deploy that forgot the manual path would quietly switch billing on for a deployment whose operator had chosen to run without it, exposing a live Stripe webhook endpoint and a paid tier ladder on a service configured for neither. Production has been running a non-billing appview since May precisely because someone did not use `make build-trixie`. Billing is now opt-in and off by default: make build-trixie # no billing make build-trixie BILLING=1 # billing deploy/upcloud update appview # no billing deploy/upcloud update appview --with-billing Only the appview is affected; hold, scanner, labeler and the credential helper never reference pkg/billing. The flag alone is not enough, so verifyAppviewBilling reads the built binary before anything is uploaded and refuses to ship a mismatch. It observes rather than trusts, because the flag and the artifact can disagree for reasons the flag cannot see: a stale bin/atcr-appview from an earlier build, a Makefile that hardcodes the tag, a builder image that ignored it. The Stripe SDK links only under the tag, so its symbols are a direct measurement — verified as a discriminator here: 2338 stripe-go strings with the tag, 0 without. A missing binary is an error rather than an absence of symbols, so a deploy cannot proceed on a file that was never built by reading it as "no billing". Verified: the default build-trixie output has 0 stripe-go symbols and does carry the "Billing is not enabled on this deployment" stub. Guard covered both directions by test, plus the missing-binary case. make lint 0 issues across root, deploy and credential-helper; make test green across 44 packages. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AV6Mk2AgghFsNo4HWQEaBV --- Makefile | 31 ++++++++++-- deploy/upcloud/provision.go | 13 +++-- deploy/upcloud/update.go | 68 ++++++++++++++++++++++++--- deploy/upcloud/update_billing_test.go | 59 +++++++++++++++++++++++ 4 files changed, 158 insertions(+), 13 deletions(-) create mode 100644 deploy/upcloud/update_billing_test.go diff --git a/Makefile b/Makefile index 51ef59b..ed1209f 100644 --- a/Makefile +++ b/Makefile @@ -71,7 +71,32 @@ build-oauth-helper: ## Build OAuth helper only # 2.43, which otherwise stamps sqrtf@GLIBC_2.43 onto cgo-linked output). TRIXIE_BUILDER_IMAGE ?= golang:1-trixie -build-trixie: generate ## Build all production binaries (appview, hold, credential-helper, scanner, labeler) for linux/amd64 in a Debian 13 (glibc 2.41) container +# Billing is OFF by default, and that default is load-bearing. +# +# pkg/billing sits behind the `billing` build tag, so the tag decides whether a +# deployment has Stripe compiled in at all. This target used to hardcode it, +# which meant the only supported way to build a non-billing appview was to +# bypass the Makefile and drive the container by hand — and every deploy that +# forgot to would silently switch billing on for a deployment whose operator +# had chosen to run without it. +# +# Opt in explicitly instead: +# +# make build-trixie # no billing (default) +# make build-trixie BILLING=1 # billing compiled in +# +# Only the appview is affected; hold, scanner, labeler and the credential +# helper never reference pkg/billing. Verify what you built with: +# +# strings -a bin/atcr-appview | grep -c stripe-go # 0 when billing is off +BILLING ?= +ifeq ($(BILLING),) +APPVIEW_TAGS := +else +APPVIEW_TAGS := -tags billing +endif + +build-trixie: generate ## Build all production binaries for linux/amd64 in a Debian 13 (glibc 2.41) container. Billing is off unless BILLING=1. @echo "→ Building in $(TRIXIE_BUILDER_IMAGE) for glibc 2.41 compatibility..." @mkdir -p bin docker run --rm \ @@ -87,12 +112,12 @@ build-trixie: generate ## Build all production binaries (appview, hold, credenti $(TRIXIE_BUILDER_IMAGE) \ bash -c '\ set -e && \ - go build -trimpath -tags billing -ldflags="-s -w $(APPVIEW_LDFLAGS)" -o bin/atcr-appview ./cmd/appview && \ + go build -trimpath $(APPVIEW_TAGS) -ldflags="-s -w $(APPVIEW_LDFLAGS)" -o bin/atcr-appview ./cmd/appview && \ go build -trimpath -ldflags="-s -w" -o bin/atcr-hold ./cmd/hold && \ (cd cmd/credential-helper/atcr && go build -trimpath -ldflags="-s -w" -o ../../../bin/docker-credential-atcr .) && \ go build -trimpath -ldflags="-s -w" -o bin/atcr-labeler ./cmd/labeler && \ cd scanner && go build -trimpath -ldflags="-s -w" -o ../bin/atcr-scanner ./cmd/scanner' - @echo "✓ Built to bin/ (glibc ≥ 2.41 compatible)" + @echo "✓ Built to bin/ (glibc ≥ 2.41 compatible), appview billing: $(if $(BILLING),ENABLED,disabled)" ##@ Test Targets diff --git a/deploy/upcloud/provision.go b/deploy/upcloud/provision.go index 381b0f5..7f2cb12 100644 --- a/deploy/upcloud/provision.go +++ b/deploy/upcloud/provision.go @@ -30,7 +30,8 @@ var provisionCmd = &cobra.Command{ s3Secret, _ := cmd.Flags().GetString("s3-secret") withScanner, _ := cmd.Flags().GetBool("with-scanner") withLabeler, _ := cmd.Flags().GetBool("with-labeler") - return cmdProvision(token, zone, plan, sshKey, s3Secret, withScanner, withLabeler) + withBilling, _ := cmd.Flags().GetBool("with-billing") + return cmdProvision(token, zone, plan, sshKey, s3Secret, withScanner, withLabeler, withBilling) }, } @@ -41,10 +42,11 @@ func init() { provisionCmd.Flags().String("s3-secret", "", "S3 secret access key (for existing object storage)") provisionCmd.Flags().Bool("with-scanner", false, "Deploy vulnerability scanner alongside hold") provisionCmd.Flags().Bool("with-labeler", false, "Deploy content moderation labeler alongside appview") + provisionCmd.Flags().Bool("with-billing", false, "Compile Stripe billing into the appview (pkg/billing, `billing` build tag). Off by default.") rootCmd.AddCommand(provisionCmd) } -func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, withLabeler bool) error { +func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, withLabeler, withBilling bool) error { cfg, err := loadConfig(zone, plan, sshKeyPath, s3Secret) if err != nil { return err @@ -373,9 +375,12 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w if appviewCreated || holdCreated { rootDir := projectRoot() - if err := runMakeBuildTrixie(rootDir); err != nil { + if err := runMakeBuildTrixie(rootDir, withBilling); err != nil { return fmt.Errorf("build: %w", err) } + if err := verifyAppviewBilling(filepath.Join(rootDir, "bin", "atcr-appview"), withBilling); err != nil { + return err + } fmt.Println("\nWaiting for cloud-init to complete on new servers...") if appviewCreated { @@ -425,7 +430,7 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w if state.LabelerEnabled && !appviewCreated { rootDir := projectRoot() labelerLocal := filepath.Join(rootDir, "bin", "atcr-labeler") - if err := runMakeBuildTrixie(rootDir); err != nil { + if err := runMakeBuildTrixie(rootDir, withBilling); err != nil { return fmt.Errorf("build labeler: %w", err) } labelerRemote := naming.InstallDir() + "/bin/" + naming.Labeler() diff --git a/deploy/upcloud/update.go b/deploy/upcloud/update.go index 0da3d97..f260219 100644 --- a/deploy/upcloud/update.go +++ b/deploy/upcloud/update.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "time" @@ -25,7 +26,8 @@ var updateCmd = &cobra.Command{ } withScanner, _ := cmd.Flags().GetBool("with-scanner") withLabeler, _ := cmd.Flags().GetBool("with-labeler") - return cmdUpdate(target, withScanner, withLabeler) + withBilling, _ := cmd.Flags().GetBool("with-billing") + return cmdUpdate(target, withScanner, withLabeler, withBilling) }, } @@ -40,13 +42,14 @@ var sshCmd = &cobra.Command{ } func init() { + updateCmd.Flags().Bool("with-billing", false, "Compile Stripe billing into the appview (pkg/billing, `billing` build tag). Off by default: a deployment that runs without billing must not gain it because someone redeployed.") updateCmd.Flags().Bool("with-scanner", false, "Enable and deploy vulnerability scanner alongside hold") updateCmd.Flags().Bool("with-labeler", false, "Enable and deploy content moderation labeler alongside appview") rootCmd.AddCommand(updateCmd) rootCmd.AddCommand(sshCmd) } -func cmdUpdate(target string, withScanner, withLabeler bool) error { +func cmdUpdate(target string, withScanner, withLabeler, withBilling bool) error { state, err := loadState() if err != nil { return err @@ -126,10 +129,20 @@ func cmdUpdate(target string, withScanner, withLabeler bool) error { // glibc 2.41 (the deploy target's glibc) regardless of the host's glibc. // build-trixie depends on the `generate` make target, which runs // `go generate ./...` on the host before invoking the trixie container. - if err := runMakeBuildTrixie(rootDir); err != nil { + if err := runMakeBuildTrixie(rootDir, withBilling); err != nil { return fmt.Errorf("build: %w", err) } + // Check what was actually produced, not what was asked for. The build tag + // is invisible in the binary's name and in its version stamp, so a wrong + // one is only discovered in production — as happened here, where a deploy + // silently compiled billing into a deployment running without it. + if slices.Contains(toUpdate, "appview") { + if err := verifyAppviewBilling(filepath.Join(rootDir, "bin", "atcr-appview"), withBilling); err != nil { + return err + } + } + // Deploy each target for _, name := range toUpdate { t := targets[name] @@ -348,9 +361,13 @@ func configValsFromState(state *InfraState) *ConfigValues { // production binaries (appview, hold, credential-helper, labeler, scanner) // inside a Debian 13 container so they link against glibc 2.41. Centralizing // the build recipe in the Makefile keeps deploy and local builds in sync. -func runMakeBuildTrixie(rootDir string) error { - fmt.Println("Running `make build-trixie` (linux/amd64, glibc 2.41)...") - cmd := exec.Command("make", "build-trixie") +func runMakeBuildTrixie(rootDir string, withBilling bool) error { + fmt.Printf("Running `make build-trixie` (linux/amd64, glibc 2.41, billing: %s)...\n", billingLabel(withBilling)) + args := []string{"build-trixie"} + if withBilling { + args = append(args, "BILLING=1") + } + cmd := exec.Command("make", args...) cmd.Dir = rootDir cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -430,3 +447,42 @@ func runSSH(ip, script string, stream bool) (string, error) { return buf.String(), fmt.Errorf("SSH command timed out after 5 minutes") } } + +func billingLabel(on bool) string { + if on { + return "ENABLED" + } + return "disabled" +} + +// verifyAppviewBilling refuses to ship an appview whose billing state is not +// the one asked for. +// +// It reads the built binary rather than trusting the flag, because the two can +// disagree for reasons the flag cannot see: a stale bin/atcr-appview left by an +// earlier build, a Makefile that hardcodes the tag, a builder image that +// ignored it. The Stripe SDK is only linked when the `billing` tag is set, so +// its symbols are a direct observation of which build this is. +// +// Getting this wrong is not cosmetic. Compiling billing into a deployment that +// runs without it exposes a live Stripe webhook endpoint and a paid tier ladder +// on a service whose operator chose neither. +func verifyAppviewBilling(binPath string, want bool) error { + data, err := os.ReadFile(binPath) + if err != nil { + return fmt.Errorf("verify appview billing: %w", err) + } + got := bytes.Contains(data, []byte("stripe-go")) + if got != want { + return fmt.Errorf( + "appview billing mismatch: asked for %s, built binary has billing %s.\n"+ + " %s carries %s Stripe symbols. Rebuild before deploying:\n"+ + " make build-trixie%s", + billingLabel(want), billingLabel(got), + binPath, map[bool]string{true: "", false: "no"}[got], + map[bool]string{true: " BILLING=1", false: ""}[want], + ) + } + fmt.Printf(" appview billing verified: %s\n", billingLabel(got)) + return nil +} diff --git a/deploy/upcloud/update_billing_test.go b/deploy/upcloud/update_billing_test.go new file mode 100644 index 0000000..a1a70d3 --- /dev/null +++ b/deploy/upcloud/update_billing_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The guard exists because the build tag is invisible in the binary's name and +// in its version stamp: a wrong one is otherwise only discovered in production. +// These cases pin the two directions that matter — shipping billing to a +// deployment that runs without it, and shipping a stub to one that expects it. +func TestVerifyAppviewBilling(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + content string + want bool + wantErr bool + }{ + {"billing build, billing wanted", "...github.com/stripe/stripe-go/v79...", true, false}, + {"stub build, no billing wanted", "...Billing is not enabled on this deployment...", false, false}, + {"billing build leaking into a no-billing deploy", "...stripe-go...", false, true}, + {"stub build where billing was asked for", "...no payment code here...", true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "atcr-appview") + if err := os.WriteFile(path, []byte(tc.content), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + err := verifyAppviewBilling(path, tc.want) + if tc.wantErr && err == nil { + t.Fatalf("want=%v: expected a mismatch error, got nil", tc.want) + } + if !tc.wantErr && err != nil { + t.Fatalf("want=%v: unexpected error: %v", tc.want, err) + } + // The message has to say which way round the mismatch is, or the + // operator cannot tell whether to add or drop the flag. + if tc.wantErr && !strings.Contains(err.Error(), "billing mismatch") { + t.Errorf("error should name the mismatch, got: %v", err) + } + }) + } +} + +// A missing binary must fail loudly rather than be read as "no stripe symbols, +// therefore billing is off" — which would let a deploy proceed on a file that +// was never built. +func TestVerifyAppviewBillingMissingBinary(t *testing.T) { + t.Parallel() + err := verifyAppviewBilling(filepath.Join(t.TempDir(), "does-not-exist"), false) + if err == nil { + t.Fatal("expected an error for a missing binary, got nil") + } +}