From b3617ed195c14ef043e7fda1a54ed37f36002c81 Mon Sep 17 00:00:00 2001 From: Jeff Gonzalez Date: Tue, 8 Sep 2026 09:22:42 -0400 Subject: [PATCH] the lock records what the source could be read as, and check audits it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Access is verified once, at fetch, and the copy is durable — so whether an adoption is still legitimate rests on the relative visibility of two repositories, which somebody can change with a checkbox a year later without ever seeing the adoption. The lock gains an optional fourth field and check turns that from a silent permanent hazard into something that runs. It costs nothing at add time, because the anonymous request already happened to decide whether to warn and the answer was being thrown away, and one request per run at check time rather than one per document, because only our own visibility has to be current. The stored value decays in both directions, so a source recorded not-public is re-probed only when the alarm would fire, and a source that has since gone public updates the lock and says nothing. Fixes a bug found while testing the alarm rather than after shipping it. sourceVisibility returned public whenever no credential was configured, which is sound at add time — the fetch had just succeeded anonymously — and wrong in the audit, where it is a probe and not a fetch: it would have silently cleared real alarms. Probing is now its own function that always asks with no credential, because what matters is what a stranger can read and not what we can. The value is recorded as public or not-public and never private: an anonymous request tells those apart and nothing finer, so it cannot see two repositories private to different people, which is the case that genuinely widens access. Three-field locks still load, and unknown visibility round-trips as absent rather than as a value. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018UTxuSizozEA8yDitPuris --- .loom/externals/.locks | 24 +++++++------ internal/external/exposure.go | 68 +++++++++++++++++++++++++++++++++++ internal/external/external.go | 57 +++++++++++++++++++++-------- internal/lock/lock.go | 42 ++++++++++++++++++---- internal/lock/lock_test.go | 57 +++++++++++++++++++++++++++++ 5 files changed, 216 insertions(+), 32 deletions(-) create mode 100644 internal/external/exposure.go create mode 100644 internal/lock/lock_test.go diff --git a/.loom/externals/.locks b/.loom/externals/.locks index 9ecd3f5..38c95a4 100644 --- a/.loom/externals/.locks +++ b/.loom/externals/.locks @@ -1,12 +1,14 @@ # loomctl locks — one record per adopted document. -# pathurletag The url is resolved: a short form would follow -# whatever the default branch is at the time you ask. -git.hypertheory-labs.dev/jeffry/homelab-cluster/gitea.md https://git.hypertheory-labs.dev/jeffry/homelab-cluster/raw/branch/main/.loom/published/gitea.md "530c5bef62bbd325956ed170bb2decf37975e4b9" -git.hypertheory-labs.dev/loom/annotating/annotating.md https://git.hypertheory-labs.dev/loom/annotating/raw/branch/main/.loom/published/annotating.md "9b1f7e6ca92f2339b2d433686c27845362944bdb" -git.hypertheory-labs.dev/loom/bedrock/loom-directory.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/loom-directory.md "ee0f49cb900c0812678061971194325d9cba366a" -git.hypertheory-labs.dev/loom/bedrock/publication.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/publication.md "c0d9231aa8eed3041c60b72da5735585457fbc90" -git.hypertheory-labs.dev/loom/bedrock/recording-decisions.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/recording-decisions.md "d190eeae7929b9a99ecf09211d94e7e5b32635bd" -git.hypertheory-labs.dev/loom/bedrock/sibling-facets.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/sibling-facets.md "a46446a34ccb8bfc533d3cce19f4c88548c4fa04" -git.hypertheory-labs.dev/loom/bedrock/starting.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/starting.md "b9eefba0f4668a496ccfc6a1377277f0721456d2" -git.hypertheory-labs.dev/loom/cart/cart.md https://git.hypertheory-labs.dev/loom/cart/raw/branch/main/.loom/published/cart.md "15331f1a9cc81bf61a44830cfbb7c274f4c2b119" -git.hypertheory-labs.dev/loom/externals/externals.md https://git.hypertheory-labs.dev/loom/externals/raw/branch/main/.loom/published/externals.md "a7586eb52caf275d9bcedbbd8042c43e5aaad0b9" +# pathurletag[visibility] +# The url is resolved: a short form would follow whatever the default branch +# is at the time you ask. visibility is what the source could be read as when +# it was fetched, because that is checked once and the copy is durable. +git.hypertheory-labs.dev/jeffry/homelab-cluster/gitea.md https://git.hypertheory-labs.dev/jeffry/homelab-cluster/raw/branch/main/.loom/published/gitea.md "530c5bef62bbd325956ed170bb2decf37975e4b9" public +git.hypertheory-labs.dev/loom/annotating/annotating.md https://git.hypertheory-labs.dev/loom/annotating/raw/branch/main/.loom/published/annotating.md "9b1f7e6ca92f2339b2d433686c27845362944bdb" public +git.hypertheory-labs.dev/loom/bedrock/loom-directory.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/loom-directory.md "ee0f49cb900c0812678061971194325d9cba366a" public +git.hypertheory-labs.dev/loom/bedrock/publication.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/publication.md "c0d9231aa8eed3041c60b72da5735585457fbc90" public +git.hypertheory-labs.dev/loom/bedrock/recording-decisions.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/recording-decisions.md "d190eeae7929b9a99ecf09211d94e7e5b32635bd" public +git.hypertheory-labs.dev/loom/bedrock/sibling-facets.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/sibling-facets.md "a46446a34ccb8bfc533d3cce19f4c88548c4fa04" public +git.hypertheory-labs.dev/loom/bedrock/starting.md https://git.hypertheory-labs.dev/loom/bedrock/raw/branch/main/.loom/published/starting.md "b9eefba0f4668a496ccfc6a1377277f0721456d2" public +git.hypertheory-labs.dev/loom/cart/cart.md https://git.hypertheory-labs.dev/loom/cart/raw/branch/main/.loom/published/cart.md "15331f1a9cc81bf61a44830cfbb7c274f4c2b119" public +git.hypertheory-labs.dev/loom/externals/externals.md https://git.hypertheory-labs.dev/loom/externals/raw/branch/main/.loom/published/externals.md "a7586eb52caf275d9bcedbbd8042c43e5aaad0b9" public diff --git a/internal/external/exposure.go b/internal/external/exposure.go new file mode 100644 index 0000000..b5ac63f --- /dev/null +++ b/internal/external/exposure.go @@ -0,0 +1,68 @@ +package external + +import ( + "fmt" + "io" + + "git.hypertheory-labs.dev/loom/loom-cli/internal/lock" +) + +// auditExposure reports documents adopted from a source that could not be read +// anonymously, into a repository that now can. +// +// Access is verified once, at fetch, and the copy is durable. Whether the +// adoption is still legitimate rests on the relative visibility of two +// repositories — a fact somebody can change with a checkbox a year later, +// without ever seeing the adoption. This is what turns that from a silent +// permanent hazard into something that runs. +func auditExposure(root string, locks *lock.Set, out io.Writer) (changed bool) { + var suspect []lock.Record + for _, r := range locks.All() { + if r.Visibility == lock.NotPublic { + suspect = append(suspect, r) + } + } + if len(suspect) == 0 { + return false + } + + // Only our own visibility has to be current, and it is one request for the + // whole run rather than one per document. + public, known := selfVisibility(root) + if known && !public { + return false // adopted private into private; nothing has widened + } + if !known { + fmt.Fprintf(out, "\n%d document(s) came from a source that needed a credential, and I cannot\n", len(suspect)) + fmt.Fprintf(out, "tell who may read this repository — no usable origin.\n") + return false + } + + // The stored value decays in both directions. A source that has since gone + // public would otherwise raise this alarm forever, so re-check — but only + // the suspects, and only when the alarm would actually fire. + var still []lock.Record + for _, r := range suspect { + // Probe with no credential: what matters is what a stranger can read, + // not what we can. + if probeAnonymous(r.URL) == lock.Public { + r.Visibility = lock.Public + locks.Put(r) + changed = true + continue + } + still = append(still, r) + } + if len(still) == 0 { + return changed + } + + fmt.Fprintf(out, "\nEXPOSURE this repository is public and holds %d document(s) adopted from\n", len(still)) + fmt.Fprintf(out, " sources that are not:\n") + for _, r := range still { + fmt.Fprintf(out, " %s\n", r.Path) + } + fmt.Fprintf(out, " Confidentiality does not travel with the copy. This was legitimate\n") + fmt.Fprintf(out, " when adopted if this repository was not public then.\n") + return changed +} diff --git a/internal/external/external.go b/internal/external/external.go index 882263b..71b78a5 100644 --- a/internal/external/external.go +++ b/internal/external/external.go @@ -119,6 +119,7 @@ func Add(root, raw, override string, out io.Writer) error { if body == nil { return fmt.Errorf("%s: unexpected 304 for a document we do not have", raw) } + vis := sourceVisibility(cfg, body.url) // A document that is here but unlocked was fetched by hand before the tool // existed. Supplying its URL is the only way it can ever be locked, because @@ -145,7 +146,7 @@ func Add(root, raw, override string, out io.Writer) error { } // Identical, so the assertion a lock makes — this local copy is the one // being served — is verified rather than assumed. - locks.Put(lock.Record{Path: rel, URL: body.url, ETag: etag}) + locks.Put(lock.Record{Path: rel, URL: body.url, ETag: etag, Visibility: vis}) if err := locks.Save(); err != nil { return err } @@ -161,14 +162,14 @@ func Add(root, raw, override string, out io.Writer) error { if err := os.WriteFile(dest, body.data, 0o644); err != nil { return err } - locks.Put(lock.Record{Path: rel, URL: body.url, ETag: etag}) + locks.Put(lock.Record{Path: rel, URL: body.url, ETag: etag, Visibility: vis}) if err := locks.Save(); err != nil { return err } fmt.Fprintf(out, "adopted %s\n", rel) fmt.Fprintf(out, " from %s\n", body.url) - warnIfNotPublic(cfg, root, body.url, out) + warnIfNotPublic(root, body.url, vis, out) notePublishedSurface(body.url, out) if etag == "" { fmt.Fprintf(out, " etag (none served — check cannot ask conditionally)\n") @@ -186,26 +187,50 @@ func Add(root, raw, override string, out io.Writer) error { // moment of adoption. The tool can see half of that — whether this fetch needed // a credential — and cannot see the other half, which is who can read the // repository the copy is landing in. It reports the half it knows. -func warnIfNotPublic(cfg *config.Config, root, raw string, out io.Writer) { +// sourceVisibility reports what a document could be read as at the moment it was +// fetched, given that the fetch had just succeeded. +// +// It is free: when no credential was configured the fetch itself was anonymous, +// so the answer is already known; when one was, the extra request is the one the +// warning needed anyway. +// +// It must not be used to re-check a document we are not fetching. "No credential +// configured" says nothing about whether a probe would succeed, and treating it +// as public there would silently clear a real alarm. +func sourceVisibility(cfg *config.Config, raw string) string { + if u, err := url.Parse(raw); err == nil && cfg.TokenFor(u.Host) == "" { + return lock.Public // it came back without a credential + } + return probeAnonymous(raw) +} + +// probeAnonymous asks, with no credential at all, whether a URL can be read. +func probeAnonymous(raw string) string { req, err := http.NewRequest(http.MethodHead, raw, nil) if err != nil { - return - } - if cfg.TokenFor(req.URL.Host) == "" { - return // no credential was used, so the fetch was already anonymous + return "" } resp, err := client.Do(req) if err != nil { - return + return "" } defer resp.Body.Close() if resp.StatusCode == http.StatusOK { + return lock.Public + } + return lock.NotPublic +} + +// warnIfNotPublic says something when adopting would widen who can read a +// document. +// +// Adopting is copying, so a document from a repository somebody may not read +// ends up in a repository they may, and the publisher loses control of it at the +// moment of adoption. +func warnIfNotPublic(root, raw, vis string, out io.Writer) { + if vis != lock.NotPublic { return } - - // The source is private. Whether that matters depends on where it is - // landing, and a warning on every adoption a private repository performs is - // noise in exactly the workflow that is legitimate. public, known := selfVisibility(root) if known && !public { fmt.Fprintf(out, " NOTE private source, and this repository is not public either.\n") @@ -219,8 +244,7 @@ func warnIfNotPublic(cfg *config.Config, root, raw string, out io.Writer) { fmt.Fprintf(out, " repository — no usable origin. Check before you commit.\n") return } - fmt.Fprintf(out, " WARN this needed a credential — anonymously it is %s — and THIS\n", resp.Status) - fmt.Fprintf(out, " repository is public.\n") + fmt.Fprintf(out, " WARN this needed a credential, and THIS repository is public.\n") fmt.Fprintf(out, " Confidentiality does not travel with the copy: adopting this\n") fmt.Fprintf(out, " publishes it to everyone. Do not adopt from a source less\n") fmt.Fprintf(out, " readable than the repository you are adopting into.\n") @@ -307,6 +331,8 @@ func Check(root string, out io.Writer) error { staged += did results = append(results, st) } + auditPrinted := &strings.Builder{} + auditExposure(root, locks, auditPrinted) if err := locks.Save(); err != nil { return err } @@ -327,6 +353,7 @@ func Check(root string, out io.Writer) error { if staged > 0 { fmt.Fprintf(out, "\n%d staged in %s — apply or discard; nothing here drifts into being kept.\n", staged, PoladDir) } + io.WriteString(out, auditPrinted.String()) return nil } diff --git a/internal/lock/lock.go b/internal/lock/lock.go index 1b0e3a7..7954630 100644 --- a/internal/lock/lock.go +++ b/internal/lock/lock.go @@ -25,8 +25,10 @@ const Dir = ".loom/externals" const File = ".locks" const header = "# loomctl locks — one record per adopted document.\n" + - "# pathurletag The url is resolved: a short form would follow\n" + - "# whatever the default branch is at the time you ask.\n" + "# pathurletag[visibility]\n" + + "# The url is resolved: a short form would follow whatever the default branch\n" + + "# is at the time you ask. visibility is what the source could be read as when\n" + + "# it was fetched, because that is checked once and the copy is durable.\n" // Record is one adopted document. type Record struct { @@ -38,8 +40,28 @@ type Record struct { URL string // ETag is the publisher's, verbatim, including its quotes. ETag string + // Visibility is what the source could be read as when it was fetched: + // "public", "not-public", or empty for locks written before this was + // recorded. + // + // It is deliberately coarse. An anonymous request tells public from + // not-public and nothing finer, so this cannot distinguish two repositories + // private to different people — which is the case where adopting between + // private repositories genuinely widens access. + // + // It is recorded because access is checked once, at fetch, and the copy is + // durable. Whether an adoption is still legitimate depends on the relative + // visibility of two repositories, which somebody can change with a checkbox + // a year later without ever seeing the adoption. + Visibility string } +// Visibility values. Never "private": the signal cannot support the word. +const ( + Public = "public" + NotPublic = "not-public" +) + // Set is every lock, keyed by path. type Set struct { file string @@ -75,10 +97,14 @@ func LoadFile(file string) (*Set, error) { continue } parts := strings.Split(line, "\t") - if len(parts) != 3 { - return nil, fmt.Errorf("%s:%d: want 3 tab-separated fields, got %d", file, n, len(parts)) + if len(parts) != 3 && len(parts) != 4 { + return nil, fmt.Errorf("%s:%d: want 3 or 4 tab-separated fields, got %d", file, n, len(parts)) } - s.recs[parts[0]] = Record{Path: parts[0], URL: parts[1], ETag: parts[2]} + r := Record{Path: parts[0], URL: parts[1], ETag: parts[2]} + if len(parts) == 4 { + r.Visibility = parts[3] + } + s.recs[parts[0]] = r } return s, sc.Err() } @@ -112,7 +138,11 @@ func (s *Set) Save() error { var b strings.Builder b.WriteString(header) for _, r := range s.All() { - fmt.Fprintf(&b, "%s\t%s\t%s\n", r.Path, r.URL, r.ETag) + if r.Visibility == "" { + fmt.Fprintf(&b, "%s\t%s\t%s\n", r.Path, r.URL, r.ETag) + continue + } + fmt.Fprintf(&b, "%s\t%s\t%s\t%s\n", r.Path, r.URL, r.ETag, r.Visibility) } tmp, err := os.CreateTemp(filepath.Dir(p), ".locks-*") if err != nil { diff --git a/internal/lock/lock_test.go b/internal/lock/lock_test.go new file mode 100644 index 0000000..94f7dd4 --- /dev/null +++ b/internal/lock/lock_test.go @@ -0,0 +1,57 @@ +package lock + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Locks written before visibility was recorded have three fields, and must keep +// loading: a repository does not get to stop working because the tool learned +// something new. +func TestLoadsThreeAndFourFieldRecords(t *testing.T) { + root := t.TempDir() + p := Path(root) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + body := "# comment\n\nh/o/r/old.md\thttps://h/old\t\"1\"\n" + + "h/o/r/new.md\thttps://h/new\t\"2\"\tnot-public\n" + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + s, err := Load(root) + if err != nil { + t.Fatal(err) + } + old, _ := s.Get("h/o/r/old.md") + if old.Visibility != "" { + t.Errorf("a three-field record should have unknown visibility, got %q", old.Visibility) + } + nw, _ := s.Get("h/o/r/new.md") + if nw.Visibility != NotPublic { + t.Errorf("visibility = %q, want %q", nw.Visibility, NotPublic) + } + + // And unknown must survive a round trip rather than being written as a value. + if err := s.Save(); err != nil { + t.Fatal(err) + } + out, _ := os.ReadFile(p) + for _, line := range strings.Split(string(out), "\n") { + if strings.HasPrefix(line, "h/o/r/old.md") && strings.Count(line, "\t") != 2 { + t.Errorf("unknown visibility was written as a field: %q", line) + } + } +} + +func TestRejectsAMalformedRecord(t *testing.T) { + root := t.TempDir() + p := Path(root) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte("h/o/r/x.md\thttps://h/x\n"), 0o644) + if _, err := Load(root); err == nil { + t.Error("a two-field record should be an error, not a silently empty ETag") + } +}