Catch a credential when it syncs, not only when you share it (#162)

* refactor(secrets): lift the share-time credential rules into internal/secrets

The rules only ever ran on the rarest path a file takes. Moving them out of
internal/webapp is what lets internal/syncer run the same six rules on the
path every file takes, without inverting the dependency.

Pure move plus one addition: Label(), the six human strings that until now
lived only in the frontend's SECRET_LABELS — so 'bdrive share' stops printing
a bare rule id where the web dialog says 'an AWS access key'. Rule ids and the
rule/line JSON tags are unchanged: Browser.tsx keys off them, so they are a
wire contract.

* feat(sync): warn when a synced file looks like it holds a credential

The six share-time rules now run on the path every file takes. A file with an
AWS key in it used to ride a normal sync to the hub, to every teammate's disk
and into every future agent's context with no badge and no warning — while the
Share dialog one click later blocked that exact file.

Warn, never block: the op is journaled and pushed exactly as before. A hold arm
would mean a false positive silently parks someone's changes, and it would
break the cycle's degrade-to-offline posture.

- scan() reads the blob PutBlobFile just wrote (the bytes that were actually
  journaled), only on the branches that wrote one — an unchanged file is still
  never re-read.
- Findings persist per path in secrets-<mount>.json, merged rather than
  replaced: nearly every cycle scans zero files, and a whole-set rewrite would
  erase the warning seconds after it appeared. Fixing the file clears it.
- bdrive status grows a secrets block; the agent hook appends one advisory
  sentence. Rule ids and line numbers only, never the matched bytes.
- SaveSecrets failing logs and continues: advisory telemetry never gets a veto
  over convergence.

* docs: the credential check now runs on sync, not only on share

README, the CLI reference and project-files get the new bdrive status block
and the warn-never-block posture, with the three limits stated (checked when
it changes, first 1 MiB, writing device only). Diagrams: internal/secrets is a
package of its own in the overview, secretLog joins the sync engine, and the
share-gate class notes that it no longer owns the rules.

* test(sync): assert an unchanged file is never re-read for credentials

The check must ride the branch that already reads the file. Clearing the record
by hand and cycling proves it: a scan that re-read unchanged files would put the
finding back, and the daemon's 3-second tick would pay for it on every file.

internal/webapp/cli_e2e_test.go

@@ -862,6 +862,75 @@ func TestCLITemplateRefusals(t *testing.T) {
 	}
 }
 
+// The same six rules, on the path every file takes. `bdrive status` names a
+// synced file that looked like it held a credential when it last changed —
+// and the file synced anyway, which is the posture: warn, never block.
+func TestCLISecretsWarnOnSync(t *testing.T) {
+	e := newCLIEnv(t)
+	run := e.run
+
+	work := t.TempDir()
+	// Fabricated, AWS-shaped. Not a credential.
+	const plantedKey = "AKIAIOSFODNN7EXAMPLE"
+	if err := os.WriteFile(filepath.Join(work, "clean.md"), []byte("# Clean\n\nnothing here\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if out, err := run(work, "init", "--name", "secret-warn", "--yes"); err != nil {
+		t.Fatalf("init: %v\n%s", err, out)
+	}
+	defer run(work, "stop", work)
+
+	// Nothing planted yet: the block is absent entirely, not empty.
+	out, err := run(work, "status", work)
+	if err != nil {
+		t.Fatalf("status: %v\n%s", err, out)
+	}
+	if strings.Contains(out, "secrets:") {
+		t.Fatalf("status names credentials with none found:\n%s", out)
+	}
+
+	if err := os.WriteFile(filepath.Join(work, "deploy.md"), []byte(
+		"# Deploy\n\nexport AWS_ACCESS_KEY_ID="+plantedKey+"\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if out, err := run(work, "sync"); err != nil {
+		t.Fatalf("sync: %v\n%s", err, out)
+	}
+	out, err = run(work, "status", work)
+	if err != nil {
+		t.Fatalf("status: %v\n%s", err, out)
+	}
+	for _, want := range []string{"secrets:", "deploy.md:3", "an AWS access key", "when they last changed"} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("status missing %q:\n%s", want, out)
+		}
+	}
+	if strings.Contains(out, plantedKey) {
+		t.Fatalf("status echoed the key back:\n%s", out)
+	}
+
+	// It synced anyway — warn, never hold. The hub has the file.
+	if out, err := run(work, "share", "deploy.md", "--force"); err != nil || !strings.Contains(out, "/s/") {
+		t.Fatalf("the flagged file did not reach the hub: %v\n%s", err, out)
+	}
+
+	// Fixing the file is the whole remedy: no command, no flag.
+	if err := os.WriteFile(filepath.Join(work, "deploy.md"), []byte(
+		"# Deploy\n\nread AWS_ACCESS_KEY_ID from the environment\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if out, err := run(work, "sync"); err != nil {
+		t.Fatalf("sync: %v\n%s", err, out)
+	}
+	out, err = run(work, "status", work)
+	if err != nil {
+		t.Fatalf("status: %v\n%s", err, out)
+	}
+	if strings.Contains(out, "secrets:") {
+		t.Fatalf("the warning outlived the credential:\n%s", out)
+	}
+}
+
 // `bdrive share` refuses a file that looks like it holds credentials, and
 // --force is the way past it. This is the flow BEA-111 exists for: the CLI
 // used to print the URL and nothing else.

internal/webapp/secrets.go

@@ -1,74 +0,0 @@
-package webapp
-
-import (
-	"bytes"
-	"regexp"
-	"sort"
-)
-
-// Minting a share link is the one place on the hub where a member turns private
-// bytes into a public URL, so it is the one place worth reading the bytes
-// first. The check is deliberately narrow: six anchored rules over the first
-// 1 MiB, at mint time only.
-//
-// It says nothing about the file tomorrow. A link serves the file's LATEST
-// content forever (see the package comment in shares.go), so every string a
-// user sees says the file was checked *at the moment you shared it* — never
-// that the file is clean.
-
-// secretScanLimit is how much of a file the share gate reads. The boundary is
-// a decision, not an accident: a key past the first MiB mints silently, which
-// is asserted in shares_test.go so nobody "fixes" it by accident.
-const secretScanLimit = 1 << 20
-
-// secretFinding is one credential-shaped string: which rule fired, and where.
-// Never the matched text — see scanSecrets.
-type secretFinding struct {
-	Rule string `json:"rule"`
-	Line int    `json:"line"`
-}
-
-var secretRules = []struct {
-	id string
-	re *regexp.Regexp
-}{
-	{"aws_access_key_id", regexp.MustCompile(`AKIA[0-9A-Z]{16}`)},
-	// The bodies below are what keep the prefixes off prose: a bare `sk-` in a
-	// sentence is not a key. If one still fires on real docs, tighten the body
-	// rather than dropping the rule — `--force` and Share anyway are the
-	// escape hatch, which is why they ship in the same change.
-	{"openai_api_key", regexp.MustCompile(`sk-[A-Za-z0-9_-]{20,}`)},
-	{"github_pat", regexp.MustCompile(`ghp_[A-Za-z0-9]{36}`)},
-	{"slack_token", regexp.MustCompile(`xox[baprs]-[A-Za-z0-9-]{10,}`)},
-	{"private_key", regexp.MustCompile(`-----BEGIN [A-Z ]*PRIVATE KEY-----`)},
-	{"gitlab_pat", regexp.MustCompile(`glpat-[A-Za-z0-9_-]{20,}`)},
-}
-
-// scanSecrets reports credential-shaped strings in buf, as rule ids and line
-// numbers ONLY. The matched text must never reach a response body, a log line,
-// or a metric label — the same argument reads.go:28-40 makes for actor
-// identity, and a 409 body is the easiest place in the codebase to leak it.
-//
-// Byte-oriented on purpose: a bufio.Scanner over a 1 MiB minified file with no
-// newline blows its 64 KiB token limit and returns nothing at all, which is a
-// check that silently passes everything.
-func scanSecrets(buf []byte) []secretFinding {
-	seen := map[secretFinding]bool{}
-	var out []secretFinding
-	for _, rule := range secretRules {
-		for _, m := range rule.re.FindAllIndex(buf, -1) {
-			f := secretFinding{Rule: rule.id, Line: bytes.Count(buf[:m[0]], []byte("\n")) + 1}
-			if !seen[f] {
-				seen[f] = true
-				out = append(out, f)
-			}
-		}
-	}
-	sort.Slice(out, func(i, j int) bool {
-		if out[i].Line != out[j].Line {
-			return out[i].Line < out[j].Line
-		}
-		return out[i].Rule < out[j].Rule
-	})
-	return out
-}

internal/webapp/secrets_test.go

@@ -1,63 +0,0 @@
-package webapp
-
-import (
-	"reflect"
-	"strings"
-	"testing"
-)
-
-// Fabricated, structurally-valid-looking strings. None is a real credential.
-const (
-	fakeAWSKey    = "AKIAIOSFODNN7EXAMPLE"
-	fakeOpenAIKey = "sk-abcdefghijklmnopqrstuvwxyz0123456789"
-	fakeGitHubPAT = "ghp_" + "abcdefghijklmnopqrstuvwxyz0123456789ab"
-	fakeSlackTok  = "xoxb-1234567890-abcdefghij"
-	fakeGitLabPAT = "glpat-abcdefghij0123456789XY"
-	fakePrivKey   = "-----BEGIN RSA PRIVATE KEY-----"
-)
-
-func TestScanSecrets(t *testing.T) {
-	for _, tc := range []struct {
-		name string
-		buf  string
-		want []secretFinding
-	}{
-		{"clean", "# Notes\n\nnothing to see, sk- is just a prefix here\n", nil},
-		{"aws", "line1\nline2\nkey = " + fakeAWSKey + "\n", []secretFinding{{"aws_access_key_id", 3}}},
-		{"openai", "OPENAI=" + fakeOpenAIKey, []secretFinding{{"openai_api_key", 1}}},
-		{"github", "\n\n" + fakeGitHubPAT, []secretFinding{{"github_pat", 3}}},
-		{"slack", "token: " + fakeSlackTok, []secretFinding{{"slack_token", 1}}},
-		{"gitlab", "x\n" + fakeGitLabPAT, []secretFinding{{"gitlab_pat", 2}}},
-		{"private key", "a\nb\nc\n" + fakePrivKey + "\nMIIE...\n", []secretFinding{{"private_key", 4}}},
-		{
-			// One line, one rule, three keys: one finding, not three.
-			"multi key line deduped",
-			"a=" + fakeAWSKey + " b=AKIAZZZZZZZZZZZZZZZZ c=AKIAYYYYYYYYYYYYYYYY",
-			[]secretFinding{{"aws_access_key_id", 1}},
-		},
-		{
-			"two rules same line",
-			"env: " + fakeAWSKey + " " + fakeSlackTok,
-			[]secretFinding{{"aws_access_key_id", 1}, {"slack_token", 1}},
-		},
-		{
-			// A bufio.Scanner would blow its 64 KiB token limit here and report
-			// nothing at all — which is why scanSecrets is byte-oriented.
-			"no newline in a big buffer",
-			strings.Repeat("x", 300_000) + fakeAWSKey,
-			[]secretFinding{{"aws_access_key_id", 1}},
-		},
-		{
-			"sorted by line then rule",
-			fakeSlackTok + "\n" + fakeAWSKey,
-			[]secretFinding{{"slack_token", 1}, {"aws_access_key_id", 2}},
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			got := scanSecrets([]byte(tc.buf))
-			if !reflect.DeepEqual(got, tc.want) {
-				t.Fatalf("scanSecrets = %v, want %v", got, tc.want)
-			}
-		})
-	}
-}

internal/webapp/shares.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/runbear-io/beardrive/internal/secrets"
 )
 
 // Share links make one file publicly readable at /s/<unguessable-token> —
@@ -289,13 +291,13 @@ func (s *Server) handleShareCreate(v *volume, w http.ResponseWriter, r *http.Req
 		// 1 MiB and close: source.Open streams from the object store, so this
 		// aborts the rest of the transfer rather than pulling a 500 MB file
 		// down to look at its first megabyte. Don't "fix" it into a ReadAll.
-		buf, err := io.ReadAll(io.LimitReader(rc, secretScanLimit))
+		buf, err := io.ReadAll(io.LimitReader(rc, secrets.ScanLimit))
 		rc.Close()
 		if err != nil {
 			storageErr(w, http.StatusServiceUnavailable, "could not read the file to check it for credentials", err)
 			return
 		}
-		if findings := scanSecrets(buf); len(findings) > 0 {
+		if findings := secrets.Scan(buf); len(findings) > 0 {
 			writeJSONStatus(w, http.StatusConflict, map[string]any{
 				"error":    "this file looks like it contains credentials",
 				"findings": findings,

internal/webapp/shares_test.go

@@ -15,6 +15,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/runbear-io/beardrive/internal/secrets"
 )
 
 func httptestNewRequestBody(method, url string, data []byte) *http.Request {
@@ -729,11 +731,11 @@ func postShare(t *testing.T, srv *Server, h http.Handler, project string, body a
 	return doHTTP(h, req)
 }
 
-func decodeFindings(t *testing.T, rec *httptest.ResponseRecorder) []secretFinding {
+func decodeFindings(t *testing.T, rec *httptest.ResponseRecorder) []secrets.Finding {
 	t.Helper()
 	var out struct {
-		Error    string          `json:"error"`
-		Findings []secretFinding `json:"findings"`
+		Error    string            `json:"error"`
+		Findings []secrets.Finding `json:"findings"`
 	}
 	if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil {
 		t.Fatalf("decode 409 body %q: %v", rec.Body, err)
@@ -755,7 +757,7 @@ func TestShareSecretScan(t *testing.T) {
 	if rec.Code != http.StatusConflict {
 		t.Fatalf("share of a file holding a key: %d %s, want 409", rec.Code, rec.Body)
 	}
-	if got := decodeFindings(t, rec); !reflect.DeepEqual(got, []secretFinding{{"aws_access_key_id", 5}}) {
+	if got := decodeFindings(t, rec); !reflect.DeepEqual(got, []secrets.Finding{{Rule: "aws_access_key_id", Line: 5}}) {
 		t.Fatalf("findings = %v, want aws_access_key_id on line 5", got)
 	}
 	if n := len(srv.Shares.List(p.ID)); n != 0 {
@@ -795,7 +797,7 @@ func TestShareSecretScan(t *testing.T) {
 }
 
 // TestShareSecretNeverEchoed is the one rule that cannot bend: the matched
-// bytes never leave scanSecrets — not in the body, not in the log.
+// bytes never leave secrets.Scan — not in the body, not in the log.
 func TestShareSecretNeverEchoed(t *testing.T) {
 	srv, p, _, f, h := shareHub(t)
 	f.put("dev1", "creds.md", "key = "+planted+"\n")