From 7b48ca2ecc43b52fb6c5bd63f339d931901d9ad0 Mon Sep 17 00:00:00 2001 From: weichen Date: Sun, 13 Sep 2026 09:32:08 +0000 Subject: [PATCH] fix(docgov): assert edit duty before lease; project via SaveDocumentContent Unauthorized actors no longer grab leases; controlled PATCH projects through DocGov so author_last_edited and audit survive, with idempotent retry on failure. Co-authored-by: Cursor --- internal/docgov/lease.go | 30 ++++++----- internal/docgov/lease_test.go | 34 ++++++++++++ internal/server/document_revision_handlers.go | 34 +++++++----- .../document_revision_patch_order_test.go | 52 ++++++++++++++++++- 4 files changed, 122 insertions(+), 28 deletions(-) diff --git a/internal/docgov/lease.go b/internal/docgov/lease.go index 779a5442..aff1163d 100644 --- a/internal/docgov/lease.go +++ b/internal/docgov/lease.go @@ -287,28 +287,32 @@ func (e *GovernanceEngine) MarkProposalApplied(ctx context.Context, proposalID s return nil } -// AssertHumanCanEdit acquires/renews the human lease and checks author duty + drafting lifecycle -// without writing document body. Used by revision PATCH before ledger mutation. +// AssertHumanCanEdit checks author duty + drafting lifecycle BEFORE acquiring/renewing +// the human lease, so unauthorized actors never become lease holders. func (e *GovernanceEngine) AssertHumanCanEdit(ctx context.Context, docID string, actorID int64, actorRole UserRole) error { + e.mu.Lock() + doc, err := e.getDocLocked(ctx, docID) + if err != nil { + e.mu.Unlock() + return err + } + if err := e.sm.CanEditDocumentContent(doc, actorID, actorRole); err != nil { + e.mu.Unlock() + return err + } + e.mu.Unlock() + holderID := fmt.Sprintf("%d", actorID) if actorID <= 0 { holderID = "human_anon" } - if _, err := e.AcquireLease(ctx, LeaseAcquireRequest{ + _, err = e.AcquireLease(ctx, LeaseAcquireRequest{ DocID: docID, HolderType: LeaseHolderHuman, HolderID: holderID, TTL: DefaultHumanLeaseTTL, - }); err != nil { - return err - } - e.mu.Lock() - defer e.mu.Unlock() - doc, err := e.getDocLocked(ctx, docID) - if err != nil { - return err - } - return e.sm.CanEditDocumentContent(doc, actorID, actorRole) + }) + return err } // SaveDocumentContent 人工正文保存：校验可编辑 + 获取/续 human 租约 + 乐观哈希 + 落盘 diff --git a/internal/docgov/lease_test.go b/internal/docgov/lease_test.go index 590ded14..6e1d4deb 100644 --- a/internal/docgov/lease_test.go +++ b/internal/docgov/lease_test.go @@ -85,3 +85,37 @@ func TestSameHolderRenewLease(t *testing.T) { t.Fatalf("expected renewed TTL, expires=%v", doc.LeaseExpiresAt) } } + +func TestAssertHumanCanEdit_ChecksDutyBeforeLease(t *testing.T) { + engine := docgov.NewGovernanceEngine(nil) + ctx := context.Background() + docID := "doc_assert_order_001" + _, err := engine.CreateOrGetDocument(ctx, docID, 1, "intake", "摸底", "V0.1", "", + 1, "作者", 2, "审核", 3, "批准") + if err != nil { + t.Fatal(err) + } + + // Non-author must fail without becoming lease holder. + if err := engine.AssertHumanCanEdit(ctx, docID, 99, docgov.RoleAuthor); err == nil { + t.Fatal("non-author AssertHumanCanEdit must fail") + } + doc, err := engine.GetDocument(ctx, docID) + if err != nil { + t.Fatal(err) + } + if doc.HasActiveLease(time.Now()) && doc.LeaseHolderID == "99" { + t.Fatal("unauthorized actor must not hold human lease") + } + + if err := engine.AssertHumanCanEdit(ctx, docID, 1, docgov.RoleAuthor); err != nil { + t.Fatalf("author AssertHumanCanEdit: %v", err) + } + doc, err = engine.GetDocument(ctx, docID) + if err != nil { + t.Fatal(err) + } + if !doc.HasActiveLease(time.Now()) || doc.LeaseHolderType != docgov.LeaseHolderHuman || doc.LeaseHolderID != "1" { + t.Fatalf("author should hold human lease after assert, got type=%s id=%s", doc.LeaseHolderType, doc.LeaseHolderID) + } +} diff --git a/internal/server/document_revision_handlers.go b/internal/server/document_revision_handlers.go index adc4f145..7587a733 100644 --- a/internal/server/document_revision_handlers.go +++ b/internal/server/document_revision_handlers.go @@ -238,9 +238,26 @@ func (s *Server) handleDocRevisionPatchContent(w http.ResponseWriter, r *http.Re return } - // 3) Project fact-source AFTER ledger commit. Projection failure returns error (no fake 200). - // Ledger remains authoritative; client should retry projection via re-save or admin repair. - if kind == "controlled" || workDir != "" { + // 3) Project fact-source AFTER ledger commit. Controlled docs use SaveDocumentContent so + // author_last_edited / audit trail stay consistent; failure → DOCUMENT_PROJECTION_FAILED + // (ledger remains authoritative; same Idempotency-Key retry re-projects). + if kind == "controlled" && s.svc != nil && s.svc.DocGov() != nil { + if _, err := s.svc.DocGov().SaveDocumentContent(r.Context(), docgov.ContentSaveRequest{ + DocID: docID, + ActorID: user.ID, + ActorRole: docgov.RoleAuthor, + ActorName: user.Name, + MarkdownBody: body.Markdown, + }); err != nil { + writeJSONResponse(w, http.StatusInternalServerError, map[string]any{ + "code": "DOCUMENT_PROJECTION_FAILED", + "error": err.Error(), + "hint": "账本已提交但 DocGov 正文/审计投影失败；请用同一 Idempotency-Key 重试保存。账本 head 仍为权威。", + "state": res.State, + }) + return + } + } else if workDir != "" { if err := rev.ProjectOutputMarkdown(docID, s.dataRoot()); err != nil { writeJSONResponse(w, http.StatusInternalServerError, map[string]any{ "code": "DOCUMENT_PROJECTION_FAILED", @@ -250,17 +267,6 @@ func (s *Server) handleDocRevisionPatchContent(w http.ResponseWriter, r *http.Re }) return } - if s.svc != nil && s.svc.DocGov() != nil { - if err := s.svc.DocGov().MarkDocumentContentUpdated(r.Context(), docID, res.State.HeadSHA256, false); err != nil { - writeJSONResponse(w, http.StatusInternalServerError, map[string]any{ - "code": "DOCUMENT_PROJECTION_FAILED", - "error": err.Error(), - "hint": "账本已提交但 document_records 哈希投影失败；请重试保存。", - "state": res.State, - }) - return - } - } } if kind == "freeform" && s.svc != nil && s.svc.FreeformDoc() != nil { md := body.Markdown diff --git a/internal/server/document_revision_patch_order_test.go b/internal/server/document_revision_patch_order_test.go index 87996626..558951b2 100644 --- a/internal/server/document_revision_patch_order_test.go +++ b/internal/server/document_revision_patch_order_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strconv" "testing" + "time" "chat-agent-system/internal/docrevision" ) @@ -19,7 +20,7 @@ func TestDocRevision_PatchOrder_AssertBeforeLedger_ProjectionFailureKeepsLedger( ctx := context.Background() dataRoot := srv.dataRoot() - // WorkDir whose parent is a regular file → MkdirAll fails after ledger commit. + // WorkDir whose parent is a regular file → SaveDocumentContent MkdirAll fails after ledger commit. blocker := filepath.Join(dataRoot, "proj-block") if err := os.WriteFile(blocker, []byte("not-a-dir"), 0o644); err != nil { t.Fatal(err) @@ -100,4 +101,53 @@ func TestDocRevision_PatchOrder_LeaseDeniedDoesNotAdvanceLedger(t *testing.T) { if after.HeadRevisionNo != before { t.Fatalf("ledger must not advance when AssertHumanCanEdit fails: before=%d after=%d", before, after.HeadRevisionNo) } + doc, err := srv.svc.DocGov().GetDocument(ctx, docID) + if err != nil { + t.Fatal(err) + } + if doc.HasActiveLease(time.Now()) && doc.LeaseHolderID == "1" { + t.Fatal("failed AssertHumanCanEdit must not leave non-author admin holding the lease") + } +} + +func TestDocRevision_PatchOrder_SaveDocumentContentSetsAuthorAudit(t *testing.T) { + srv := newTestServer(t) + ctx := context.Background() + workDir := filepath.Join(srv.dataRoot(), "ws-audit") + _ = os.MkdirAll(workDir, 0o755) + docID := "doc_proj1_patch_audit" + + if _, err := srv.svc.DocGov().CreateOrGetDocument(ctx, docID, 1, "intake", "摸底", "V0.1", workDir, 1, "admin", 2, "r", 3, "a"); err != nil { + t.Fatal(err) + } + st, err := srv.svc.DocRevision().OpenTemplateVersion(ctx, docID, "# base\n\nbody\n", docrevision.ActorSnapshot{DisplayName: "t"}, 1, workDir) + if err != nil { + t.Fatal(err) + } + + token := loginTestAdmin(t, srv) + md := "# base\n\nbody\n\nhuman edit\n" + req := httptest.NewRequest(http.MethodPatch, "/api/v1/documents/"+docID+"/content", + bytes.NewReader([]byte(`{"markdown":`+strconv.Quote(md)+`}`))) + withBearerAuth(req, token) + req.Header.Set("Idempotency-Key", "patch-audit-k1") + req.Header.Set("If-Match", `"`+strconv.FormatInt(st.HeadSnapshotID, 10)+":"+strconv.Itoa(st.HeadRevisionNo)+`"`) + w := httptest.NewRecorder() + srv.Routes().ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("want 200 got %d %s", w.Code, w.Body.String()) + } + doc, err := srv.svc.DocGov().GetDocument(ctx, docID) + if err != nil { + t.Fatal(err) + } + if doc.AuthorLastEditedAt == nil { + t.Fatal("SaveDocumentContent projection must set author_last_edited_at") + } + if !doc.AuthorClaimed { + t.Fatal("author_claimed should be true after human save projection") + } + if _, err := os.Stat(filepath.Join(workDir, "output.md")); err != nil { + t.Fatalf("output.md should exist after SaveDocumentContent: %v", err) + } }