2 files changed, 43 insertions(+), 3 deletions(-)

M service/read.go
M service/read_test.go

@@ 146,12 146,45 @@ func (s *Service) ResolveRev(ctx context.Context, sp *Space, rev string) (string
 // issued against. Re-issuing reads against the resolved hash instead would be
 // one extra object lookup per read for no gain, and would lose the branch name
 // from error messages.
+// ReadDocumentAtRef reads a document at an arbitrary ref, bypassing the read
+// contract's object-name requirement.
+//
+// This is the review path's entry point: rendering and diffing a proposal
+// branch genuinely needs to read one. It is deliberately a separate,
+// awkwardly-named method rather than a flag on ReadDocument, so that serving
+// unreviewed content is something a caller has to ask for by name and a reviewer
+// can grep for — never something a read surface can be talked into by a crafted
+// rev parameter.
+//
+// Do not call this from any surface that answers "read SPEC-0007".
+func (s *Service) ReadDocumentAtRef(ctx context.Context, sp *Space, ref, path string) (Document, error) {
+	if sp == nil || sp.Repo == nil {
+		return Document{}, errors.New("service: space has no open repository")
+	}
+	resolved := ref
+	if resolved == ApprovedRev {
+		resolved = sp.Repo.ApprovedBranch()
+	}
+	commit, err := sp.Repo.ResolveRev(ctx, resolved)
+	if err != nil {
+		return Document{}, readErr(err, "resolve revision %q in %s", resolved, sp.Ref)
+	}
+	d, err := sp.Repo.ReadDocument(ctx, resolved, path)
+	if err != nil {
+		return Document{}, readErr(err, "read %s at %s in %s", path, resolved, sp.Ref)
+	}
+	return Document{Path: d.Path, Blob: d.Blob.String(), Rev: commit.String(), Data: d.Data}, nil
+}
+
 func (s *Service) resolveRev(ctx context.Context, sp *Space, rev string) (plumbing.Hash, string, error) {
 	if sp == nil || sp.Repo == nil {
 		return plumbing.ZeroHash, "", errors.New("service: space has no open repository")
 	}
 	if err := ValidateReadRev(rev); err != nil {
-		return plumbing.ZeroHash, "", err
+		// Wrapped as ErrNotFound so a crafted revision cannot tell "malformed"
+		// from "absent" by probing, matching readErr's existing choice.
+		// ErrBadReadRev stays in the chain for logs and for callers that care.
+		return plumbing.ZeroHash, "", fmt.Errorf("%w: %w", ErrNotFound, err)
 	}
 	resolved := rev
 	if resolved == ApprovedRev {

@@ 71,9 71,16 @@ func TestReadDocumentDoesNotServeAProposalByDefault(t *testing.T) {
 	if !contains(approved.Data, "approved") {
 		t.Fatalf("default read returned the draft: %q", approved.Data)
 	}
-	draft, err := svc.ReadDocument(ctx, sp, "proposals/1", "specs/0007-storage.md")
+	// The read plane refuses a ref name outright — that is the guard that stops
+	// a crafted ?rev= from serving unreviewed text as approved.
+	if _, err := svc.ReadDocument(ctx, sp, "proposals/1", "specs/0007-storage.md"); !errors.Is(err, ErrBadReadRev) {
+		t.Fatalf("ReadDocument accepted a ref name: err = %v, want ErrBadReadRev", err)
+	}
+
+	// The review path reaches it deliberately, by a differently-named method.
+	draft, err := svc.ReadDocumentAtRef(ctx, sp, "proposals/1", "specs/0007-storage.md")
 	if err != nil {
-		t.Fatalf("ReadDocument at the proposal branch: %v", err)
+		t.Fatalf("ReadDocumentAtRef at the proposal branch: %v", err)
 	}
 	if !contains(draft.Data, "draft") {
 		t.Errorf("proposal read returned %q", draft.Data)