fix(config): keep the credentials file readable by its owner alone

cli/config/config.go

@@ -359,6 +359,13 @@ func WriteProfileWithExtras(path, name string, p Profile, extra map[string]any)
 	if err != nil {
 		return fmt.Errorf("config: write %s: %w", path, err)
 	}
+	// The mode above applies only to a new file. The file names private
+	// keys and data bag secrets, so tighten an existing one too, before
+	// anything is written into it; knife setups often leave it 0644.
+	if err := f.Chmod(0o600); err != nil {
+		_ = f.Close()
+		return fmt.Errorf("config: restrict %s to its owner: %w", path, err)
+	}
 	if err := toml.NewEncoder(f).Encode(raw); err != nil {
 		_ = f.Close()
 		return fmt.Errorf("config: encode %s: %w", path, err)

cli/config/config_test.go

@@ -416,6 +416,32 @@ func TestWriteProfileCreatesCredentialsFile(t *testing.T) {
 	}
 }
 
+// TestWriteProfileTightensAnExistingFile rewrites a credentials file that
+// others can read, as knife setups often leave one. The file names private
+// keys and data bag secrets, so any write leaves it readable by its owner
+// alone, whatever mode it had.
+func TestWriteProfileTightensAnExistingFile(t *testing.T) {
+	path := writeConfig(t, sampleConfig)
+	if err := os.Chmod(path, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := UpdateProfile(path, "default", func(p *Profile) error {
+		p.SSLVerifyMode = ":verify_peer"
+		return nil
+	}); err != nil {
+		t.Fatalf("UpdateProfile: %v", err)
+	}
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if perm := info.Mode().Perm(); perm != 0o600 {
+		t.Errorf("credentials mode after a rewrite = %o, want 0600", perm)
+	}
+}
+
 func TestWriteProfileRoundTripsAllKeys(t *testing.T) {
 	path := filepath.Join(t.TempDir(), ".cinc", "credentials")
 

docs/configuration.md

@@ -20,6 +20,12 @@ A leading `~` in `--config` is expanded to your home directory, so
 `--config=~/.chef/credentials` works even though no shell expands a `~`
 after `=`.
 
+The file names your signing keys and data bag secrets, so `cinc` keeps
+it readable by you alone. A new file is created with mode `0600` in a
+`0700` directory, and whenever `cinc` rewrites an existing file (with
+`cinc config create` or first-run setup) it tightens that file to
+`0600` too, even if it started out readable by others.
+
 The file holds one or more **profiles**. A profile is a named bundle of
 everything needed to talk to one server (or one Supermarket): a server
 URL, a client identity, a signing key, and a handful of optional
@@ -363,6 +369,7 @@ that:
 ```sh
 cinc config validate            # checks ~/.cinc/credentials
 cinc config validate ./creds    # checks a specific file
+cinc config validate --profile staging   # checks just one profile
 cinc config validate --format json
 ```
 

integration/suite/behaviour.go

@@ -48,6 +48,7 @@ var cliFamily = family{cases: []testCase{
 	{"cli/config-create-interactive-update", []string{"config create", "node list"}, testConfigCreateInteractiveUpdate},
 	{"cli/config-create-replace", []string{"config create", "node list"}, testConfigCreateReplace},
 	{"cli/config-create-name-collision", []string{"config create", "node list"}, testConfigCreateNameCollision},
+	{"cli/config-create-tightens-permissions", []string{"config create", "node list"}, testConfigCreateTightensPermissions},
 	{"cli/config-create-profile-name", []string{"config create"}, testConfigCreateProfileName},
 	{"cli/first-run-migrates-chef-credentials", []string{"node list", "node create", "node show"}, testFirstRunMigrates},
 	{"cli/first-run-migration-keeps-knife-keys", []string{"node list"}, testFirstRunMigrationKeepsKnifeKeys},

integration/suite/configcreate.go

@@ -3,6 +3,7 @@ package suite
 import (
 	"os"
 	"path/filepath"
+	"runtime"
 	"slices"
 	"strings"
 	"testing"
@@ -91,6 +92,27 @@ func testConfigCreateSymlinkedFile(t *testing.T, tgt Target, c *cli) {
 	c.run("node", "list", "--profile", "fresh")
 }
 
+// testConfigCreateTightensPermissions updates a profile in a credentials
+// file others can read, as knife setups often leave it. The file names
+// private keys and secrets, so config create leaves it readable by its
+// owner alone.
+func testConfigCreateTightensPermissions(t *testing.T, tgt Target, c *cli) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows has no owner-only file mode to check")
+	}
+	if err := os.Chmod(c.credentialsPath(), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	c.run("config", "create", "--profile", "extra", "--server-url", c.orgURL(tgt.Org),
+		"--client-name", tgt.Admin, "--client-key", tgt.KeyPath)
+	info, err := os.Stat(c.credentialsPath())
+	if err != nil {
+		t.Fatal(err)
+	}
+	wantEqual(t, "credentials file mode after config create", info.Mode().Perm(), os.FileMode(0o600))
+	c.run("node", "list", "--profile", "extra")
+}
+
 // testConfigCreateNameCollision adds a profile under a name the file
 // already has. Yes to "Update it instead?" updates that profile in place,
 // offering its current values; no asks for another name.