carlos / platform Public

carlos deploy: refuse a git-derived version from a dirty tree

-version defaults to the git short sha, so a dirty tree ships a stamp
naming a commit the artifact was not built from — the false-verified shape
roll-console.sh has always refused. An explicit -version is the operator
naming the release themselves and is left alone.
Paul Campbell pushed by paul@keymail.dev 00152b352c2f3f457bc18350b8500a7c360c14bf parent 3d75cb2
2 files changed, +83 −0
  • deploy.go +29 −0
  • deploy_test.go +54 −0
diff --git a/deploy.go b/deploy.go
index 1fd40ee14..a57693017 100644
--- a/deploy.go
+++ b/deploy.go
@@ -7,6 +7,7 @@
package main
import (
+ "bytes"
"context"
"errors"
"flag"
@@ -49,9 +50,13 @@ func runDeploy(args []string) {
"[-account <sqid-or-name>] [-account-id <sqid>] [-console <origin>] [-capabilities <file.json>] " +
"[-channel <ch>] [-host <host>] [-timeout 120s] <files-or-site-dir>\n" +
"or, in a project with saved defaults (or at a terminal, to create them): carlos deploy"
+ explicitVersion := *ver
if *ver == "" {
*ver = gitShortHead()
}
+ if err := deployPreflight(".", explicitVersion); err != nil {
+ log.Fatal(err)
+ }
// A bad capability file is still refused before anything ships — but the
// app name may be left to resolution now, so with an omitted -app the
// read happens again once the name is known (ship's rule).
@@ -932,6 +937,30 @@ func requireDeployAPISession(consoleFlag string) *apiSession {
return sess
}
+// deployPreflight refuses the one combination that produces a release whose
+// name is a lie: a version DERIVED from git (the -version default) taken from
+// a tree with uncommitted changes. The sha then names a commit the artifact
+// was not built from, and every later "which build is this" answer —
+// including the honest ones — inherits the lie.
+//
+// An explicit -version is untouched: that is the operator naming the release
+// themselves, and second-guessing it would break every workflow that versions
+// by something other than a sha.
+func deployPreflight(dir, explicitVersion string) error {
+ if explicitVersion != "" {
+ return nil
+ }
+ out, err := exec.Command("git", "-C", dir, "status", "--porcelain").Output()
+ if err != nil {
+ return nil // not a git checkout, or no git: nothing to be dirty
+ }
+ if len(bytes.TrimSpace(out)) == 0 {
+ return nil
+ }
+ return errors.New("this tree has uncommitted changes and -version defaults to the git short sha, " +
+ "which would name a commit this build does not match — commit, or pass -version explicitly")
+}
+
// gitShortHead resolves -version's default: the working directory's short
// git SHA. Fatal here (rather than a returned error) matches every other
// "this environment is wrong" refusal in this binary — there is no sane
diff --git a/deploy_test.go b/deploy_test.go
index 3ee25ee4d..bfa6da928 100644
--- a/deploy_test.go
+++ b/deploy_test.go
@@ -1699,3 +1699,57 @@ func TestDeployRefusesAMalformedProbeAndFallsBackToTheHeader(t *testing.T) {
t.Fatalf("deploy must still complete on the header watch:\n%s", out)
}
}
+
+// initDirtyGitRepo builds a git repo in a fresh temp dir with one committed
+// file, then modifies that file so the working tree is dirty. Skips the test
+// if git is unavailable in this environment.
+func initDirtyGitRepo(t *testing.T) string {
+ t.Helper()
+ if _, err := exec.LookPath("git"); err != nil {
+ t.Skip("git not available")
+ }
+ dir := t.TempDir()
+ run := func(args ...string) {
+ t.Helper()
+ cmd := exec.Command("git", args...)
+ cmd.Dir = dir
+ cmd.Env = append(os.Environ(),
+ "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com",
+ "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com")
+ if out, err := cmd.CombinedOutput(); err != nil {
+ t.Fatalf("git %v: %v\n%s", args, err, out)
+ }
+ }
+ run("init")
+ run("config", "user.email", "test@example.com")
+ run("config", "user.name", "test")
+ f := filepath.Join(dir, "file.txt")
+ if err := os.WriteFile(f, []byte("committed\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ run("add", "file.txt")
+ run("commit", "-m", "initial")
+ if err := os.WriteFile(f, []byte("dirty\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ return dir
+}
+
+// TestDeployRefusesADerivedVersionFromADirtyTree: the default -version is the
+// git short sha, so a dirty tree ships a stamp naming a commit the binary was
+// not built from — which is precisely how a false "verified" happens, and why
+// roll-console.sh refused it. An EXPLICIT -version is the operator saying they
+// mean it and is not second-guessed.
+func TestDeployRefusesADerivedVersionFromADirtyTree(t *testing.T) {
+ dir := initDirtyGitRepo(t)
+ err := deployPreflight(dir, "" /* no explicit -version */)
+ if err == nil {
+ t.Fatal("a dirty tree with a derived version was accepted")
+ }
+ if !strings.Contains(err.Error(), "-version") {
+ t.Fatalf("refusal %q should name the flag that overrides it", err)
+ }
+ if err := deployPreflight(dir, "abc1234"); err != nil {
+ t.Fatalf("an explicit -version from a dirty tree = %v, want accepted", err)
+ }
+}