Merge pull request 'Close the W6 review MINOR findings' (#242) from fix/w6-review-minors into main
This commit was merged in pull request #242.
This commit is contained in:
@@ -119,8 +119,19 @@ func (s *projectContextSlot) readEvidence(ctx context.Context, req extension.Pro
|
||||
}
|
||||
|
||||
func (s *projectContextSlot) recheck(ctx context.Context, req extension.ProjectContextSourceRecheckRequest) (extension.ProjectContextSourceRecheckResult, error) {
|
||||
if s == nil || req.Scope.Actor == "" || req.Scope.ProjectID == "" || req.Scope.Goal != "" {
|
||||
return extension.ProjectContextSourceRecheckResult{}, errors.New("extension: trusted source recheck scope missing")
|
||||
// Each unusable scope is reported on its own so host-side logs name the
|
||||
// actual defect; every branch still rejects (fail-closed).
|
||||
if s == nil {
|
||||
return extension.ProjectContextSourceRecheckResult{}, errors.New("extension: project context source recheck slot missing")
|
||||
}
|
||||
if req.Scope.Actor == "" {
|
||||
return extension.ProjectContextSourceRecheckResult{}, errors.New("extension: trusted source recheck scope has no actor")
|
||||
}
|
||||
if req.Scope.ProjectID == "" {
|
||||
return extension.ProjectContextSourceRecheckResult{}, errors.New("extension: trusted source recheck scope has no project")
|
||||
}
|
||||
if req.Scope.Goal != "" {
|
||||
return extension.ProjectContextSourceRecheckResult{}, errors.New("extension: source recheck scope carries a generation goal")
|
||||
}
|
||||
s.mu.RLock()
|
||||
p, holder := s.provider, s.holder
|
||||
|
||||
@@ -27,6 +27,45 @@ func registeredRecheckSlot(t *testing.T, recheck func(context.Context, extension
|
||||
return slot
|
||||
}
|
||||
|
||||
func TestProjectSourceRecheckScopeErrorsAreDistinguishable(t *testing.T) {
|
||||
var nilSlot *projectContextSlot
|
||||
slot := registeredRecheckSlot(t, func(context.Context, extension.ProjectContextSourceRecheckRequest) (extension.ProjectContextSourceRecheckResult, error) {
|
||||
t.Error("unusable recheck scope reached the provider")
|
||||
return extension.ProjectContextSourceRecheckResult{}, nil
|
||||
})
|
||||
cases := map[string]func() (extension.ProjectContextSourceRecheckResult, error){
|
||||
"slot missing": func() (extension.ProjectContextSourceRecheckResult, error) {
|
||||
return nilSlot.recheck(context.Background(), extension.ProjectContextSourceRecheckRequest{Scope: extension.ProjectContextRequest{Actor: "alice", ProjectID: "p1"}})
|
||||
},
|
||||
"no actor": func() (extension.ProjectContextSourceRecheckResult, error) {
|
||||
return slot.recheck(context.Background(), extension.ProjectContextSourceRecheckRequest{Scope: extension.ProjectContextRequest{ProjectID: "p1"}})
|
||||
},
|
||||
"no project": func() (extension.ProjectContextSourceRecheckResult, error) {
|
||||
return slot.recheck(context.Background(), extension.ProjectContextSourceRecheckRequest{Scope: extension.ProjectContextRequest{Actor: "alice"}})
|
||||
},
|
||||
"carries goal": func() (extension.ProjectContextSourceRecheckResult, error) {
|
||||
return slot.recheck(context.Background(), extension.ProjectContextSourceRecheckRequest{Scope: extension.ProjectContextRequest{Actor: "alice", ProjectID: "p1", Goal: "secret"}})
|
||||
},
|
||||
}
|
||||
seen := map[string]string{}
|
||||
for name, call := range cases {
|
||||
result, err := call()
|
||||
if err == nil {
|
||||
t.Fatalf("%s scope accepted", name)
|
||||
}
|
||||
if result.Bundle.Status != "" || result.Bundle.ProjectID != "" {
|
||||
t.Fatalf("%s scope returned a non-empty bundle: %+v", name, result.Bundle)
|
||||
}
|
||||
if other, dup := seen[err.Error()]; dup {
|
||||
t.Fatalf("%s and %s share one error message: %q", other, name, err)
|
||||
}
|
||||
seen[err.Error()] = name
|
||||
}
|
||||
if len(seen) != len(cases) {
|
||||
t.Fatalf("expected %d distinct errors, got %d: %v", len(cases), len(seen), seen)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProjectSourceRecheckFailsClosedAtThreeSecondCeiling(t *testing.T) {
|
||||
var remaining time.Duration
|
||||
slot := registeredRecheckSlot(t, func(ctx context.Context, req extension.ProjectContextSourceRecheckRequest) (extension.ProjectContextSourceRecheckResult, error) {
|
||||
|
||||
@@ -127,7 +127,10 @@ func (c *runCtx) recheckManifest(ctx context.Context, m extension.ProjectContext
|
||||
}
|
||||
expectedRefs[ref.ID] = ref
|
||||
}
|
||||
if len(current.Items) != len(m.Included) || len(current.Evidence) != len(expectedRefs) || !slices.Equal(current.Missing, m.Missing) {
|
||||
// Set-wise, not provider-order-wise: a benign reordering of the same
|
||||
// recorded identities is tolerated, while any added, dropped or replaced
|
||||
// entry still fails closed. Order carries no authorization meaning.
|
||||
if len(current.Items) != len(m.Included) || len(current.Evidence) != len(expectedRefs) || !sameMissingSet(current.Missing, m.Missing) {
|
||||
return errors.New("project source recheck returned an incomplete source set")
|
||||
}
|
||||
// Each returned item must account for exactly one recorded input; the hash
|
||||
@@ -180,9 +183,27 @@ func (c *runCtx) recheckManifest(ctx context.Context, m extension.ProjectContext
|
||||
return errors.New("project evidence permission or content changed")
|
||||
}
|
||||
}
|
||||
// Load-bearing fail-closed close: a context cancelled or expired after the
|
||||
// last structural pass above must still reject. Do not relax this to
|
||||
// `return nil`; a late cancellation would otherwise admit machine context
|
||||
// that was never rechecked against live authorization.
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
// sameMissingSet compares the rechecked and recorded missing-source lists as
|
||||
// sets. Neither input is mutated; any difference other than order is a
|
||||
// rejection, because a withdrawn or newly missing source changes what the
|
||||
// original model request actually saw.
|
||||
func sameMissingSet(current, recorded []string) bool {
|
||||
if len(current) != len(recorded) {
|
||||
return false
|
||||
}
|
||||
left, right := slices.Clone(current), slices.Clone(recorded)
|
||||
slices.Sort(left)
|
||||
slices.Sort(right)
|
||||
return slices.Equal(left, right)
|
||||
}
|
||||
|
||||
func (c *runCtx) contextRequest() extension.ProjectContextRequest {
|
||||
c.projectContextMu.Lock()
|
||||
defer c.projectContextMu.Unlock()
|
||||
|
||||
@@ -314,6 +314,103 @@ func TestProjectStepOutputRejectsUnsettledAttemptInSameStep(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestProjectSourceRecheckComparesMissingSourcesAsASet(t *testing.T) {
|
||||
f := newRecheckFixture(t)
|
||||
receipt := f.receipt
|
||||
receipt.Missing = []string{"source-b", "source-a"}
|
||||
// Order carries no authorization meaning, so a reordered list of the same
|
||||
// unresolved sources stays a pass.
|
||||
reordered := f.current
|
||||
reordered.Missing = []string{"source-a", "source-b"}
|
||||
f.answer(reordered)
|
||||
if err := f.rc.recheckManifest(context.Background(), receipt, "draft"); err != nil {
|
||||
t.Fatalf("reordered missing sources treated as a changed source set: %v", err)
|
||||
}
|
||||
if strings.Join(receipt.Missing, ",") != "source-b,source-a" || strings.Join(reordered.Missing, ",") != "source-a,source-b" {
|
||||
t.Fatalf("missing-source comparison mutated a caller slice: recorded=%q current=%q", receipt.Missing, reordered.Missing)
|
||||
}
|
||||
// Any difference other than order still fails closed.
|
||||
for name, missing := range map[string][]string{
|
||||
"dropped": {"source-a"},
|
||||
"added": {"source-a", "source-b", "source-c"},
|
||||
"replaced": {"source-a", "source-z"},
|
||||
"masked-by-dup": {"source-a", "source-a"},
|
||||
"withdrawn": nil,
|
||||
} {
|
||||
changed := f.current
|
||||
changed.Missing = missing
|
||||
f.answer(changed)
|
||||
if err := f.rc.recheckManifest(context.Background(), receipt, "draft"); err == nil {
|
||||
t.Fatalf("changed missing-source set %q was accepted", name)
|
||||
}
|
||||
}
|
||||
f.answer(reordered)
|
||||
if err := f.rc.recheckManifest(context.Background(), receipt, "draft"); err != nil {
|
||||
t.Fatalf("unrelated failure left the authorized baseline broken: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProjectStepOutputRejectsEvidenceRefWithChangedHash(t *testing.T) {
|
||||
st := openRecoveryStore(t)
|
||||
m := newRecoveryManager(t, st)
|
||||
conv := recoveryConversation(t, st)
|
||||
run := recoveryRun(t, st, conv, core.RunPending)
|
||||
m.SetProjectWorkAuthorization(func(context.Context, string, core.ID) (bool, error) { return true, nil })
|
||||
ref := extension.ProjectEvidenceRef{ID: "ref-a", SourceID: "doc-a", Revision: "r1", Hash: "hd"}
|
||||
evidenceHash := ref.Hash
|
||||
bundle := func() extension.ProjectContextBundle {
|
||||
return extension.ProjectContextBundle{Status: extension.ProjectContextReady, ProjectID: run.ProjectID, Version: "v1", Watermark: "w1",
|
||||
AuthorizationEpoch: "duty-1", PublicationEpoch: "pub-1",
|
||||
Items: []extension.ProjectContextItem{{SourceID: "source-a", Revision: "r1", Hash: "ha", Text: "AUTHORIZED_A", Required: true}},
|
||||
Evidence: []extension.ProjectEvidenceRef{{ID: ref.ID, SourceID: ref.SourceID, Revision: ref.Revision, Hash: evidenceHash}}}
|
||||
}
|
||||
m.SetProjectContextResolver(func(context.Context, extension.ProjectContextRequest) (extension.ProjectContextBundle, error) {
|
||||
return bundle(), nil
|
||||
})
|
||||
m.SetProjectContextSourceRechecker(func(context.Context, extension.ProjectContextSourceRecheckRequest) (extension.ProjectContextSourceRecheckResult, error) {
|
||||
return extension.ProjectContextSourceRecheckResult{Bundle: bundle()}, nil
|
||||
})
|
||||
m.SetProjectEvidenceReader(func(_ context.Context, req extension.ProjectEvidenceRequest) (extension.ProjectEvidence, error) {
|
||||
return extension.ProjectEvidence{Reference: req.Reference, Text: "DETAIL_A"}, nil
|
||||
})
|
||||
rc := &runCtx{mgr: m, run: run, ctx: context.Background()}
|
||||
current, _, err := rc.ProjectContext(context.Background(), "draft", 900)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, receipt, err := projectcontext.Assemble(current, 900)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
receipt.InputSHA256, receipt.AttemptID, receipt.WorkflowStepAttemptID = "input-1", "model-1", "engine-run/draft/1"
|
||||
receipt.ReadEvidence = []extension.ProjectContextManifestEntry{{ReferenceID: ref.ID, SourceID: ref.SourceID, Revision: ref.Revision, SourceHash: ref.Hash,
|
||||
InputSHA256: fmt.Sprintf("%x", sha256.Sum256([]byte("DETAIL_A")))}}
|
||||
scoped := projectcontext.WithResolveScope(context.Background(), "draft", "")
|
||||
for _, delivery := range []string{"attempted", "responded"} {
|
||||
receipt.Delivery = delivery
|
||||
if err := rc.SaveContextManifest(scoped, receipt); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
output := []byte("AUTHORIZED_A report")
|
||||
if err := rc.BindProjectStepOutput(context.Background(), "draft", "engine-run/draft/1", "prompt", time.Now().Add(-time.Second), output); err != nil {
|
||||
t.Fatalf("unchanged evidence source rejected at binding: %v", err)
|
||||
}
|
||||
if err := rc.VerifyProjectStepOutput(context.Background(), "draft", "engine-run/draft/1", "prompt", output); err != nil {
|
||||
t.Fatalf("unchanged evidence source rejected at the critic boundary: %v", err)
|
||||
}
|
||||
// Same ReferenceID and revision, different source hash: a re-published or
|
||||
// substituted document must not inherit the recorded grant.
|
||||
evidenceHash = "hd2"
|
||||
if err := rc.VerifyProjectStepOutput(context.Background(), "draft", "engine-run/draft/1", "prompt", output); err == nil {
|
||||
t.Fatal("same reference id with a changed source hash passed the critic boundary")
|
||||
}
|
||||
evidenceHash = ref.Hash
|
||||
if err := rc.VerifyProjectStepOutput(context.Background(), "draft", "engine-run/draft/1", "prompt", output); err != nil {
|
||||
t.Fatalf("restoring the recorded hash did not restore the verdict: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSaveContextManifestRejectsUnrecheckableModelInput(t *testing.T) {
|
||||
st := openRecoveryStore(t)
|
||||
m := newRecoveryManager(t, st)
|
||||
|
||||
@@ -40,6 +40,67 @@ func TestContextManifestTerminalRequiresMatchingAttempt(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestContextManifestAttemptIsNotReusableAcrossProjectOrOwner(t *testing.T) {
|
||||
st, err := OpenSQLite(filepath.Join(t.TempDir(), "attempt-identity.db"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer st.Close()
|
||||
base := extension.ProjectContextManifest{ProjectID: "p1", AttemptID: "attempt-1", InputSHA256: "input-hash", Delivery: "attempted"}
|
||||
if err := st.AppendContextManifest("alice", "run-1", base); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
base.Delivery = "responded"
|
||||
if err := st.AppendContextManifest("alice", "run-1", base); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// A terminal receipt is matched against the whole persisted identity
|
||||
// (attempt, owner, target, project, input hash), so the same attempt id
|
||||
// cannot claim a trusted response under another project or owner.
|
||||
crossProject := base
|
||||
crossProject.ProjectID = "p2"
|
||||
if err := st.AppendContextManifest("alice", "run-1", crossProject); err == nil {
|
||||
t.Fatal("attempt id reused across project ids to claim a trusted response")
|
||||
}
|
||||
crossOwner := base
|
||||
if err := st.AppendContextManifest("bob", "run-1", crossOwner); err == nil {
|
||||
t.Fatal("attempt id reused across owners to claim a trusted response")
|
||||
}
|
||||
// Planting the send under a foreign scope is refused too: attempt identity
|
||||
// is global, so one trusted send has exactly one receipt pair.
|
||||
if err := st.AppendContextManifest("alice", "run-1", base); err == nil {
|
||||
t.Fatal("attempt id reused across projects to open a second send")
|
||||
}
|
||||
if err := st.AppendContextManifest("bob", "run-1", base); err == nil {
|
||||
t.Fatal("attempt id reused across owners to open a second send")
|
||||
}
|
||||
kept, err := st.ListContextManifests("alice", "run-1")
|
||||
if err != nil || len(kept) != 2 {
|
||||
t.Fatalf("refused cross-scope receipts changed the trusted set: %+v err=%v", kept, err)
|
||||
}
|
||||
for _, m := range kept {
|
||||
if m.ProjectID != "p1" || m.AttemptID != "attempt-1" {
|
||||
t.Fatalf("foreign scope leaked into the trusted receipts: %+v", m)
|
||||
}
|
||||
}
|
||||
if foreign, err := st.ListContextManifests("bob", "run-1"); err != nil || len(foreign) != 0 {
|
||||
t.Fatalf("cross-owner attempt became visible: %+v err=%v", foreign, err)
|
||||
}
|
||||
// A genuinely different attempt in another owner/project still works, so
|
||||
// the refusals above are identity-based rather than scope-based.
|
||||
other := extension.ProjectContextManifest{ProjectID: "p2", AttemptID: "attempt-2", InputSHA256: "other-input", Delivery: "attempted"}
|
||||
if err := st.AppendContextManifest("bob", "run-2", other); err != nil {
|
||||
t.Fatalf("a distinct attempt in another project/owner was refused: %v", err)
|
||||
}
|
||||
other.Delivery = "responded"
|
||||
if err := st.AppendContextManifest("bob", "run-2", other); err != nil {
|
||||
t.Fatalf("a distinct attempt could not record its own response: %v", err)
|
||||
}
|
||||
if mine, err := st.ListContextManifests("alice", "run-2"); err != nil || len(mine) != 0 {
|
||||
t.Fatalf("another owner's response became visible: %+v err=%v", mine, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestContextManifestDoesNotPersistWorkflowOutputInResolveScope(t *testing.T) {
|
||||
st, err := OpenSQLite(filepath.Join(t.TempDir(), "scope-privacy.db"))
|
||||
if err != nil {
|
||||
|
||||
@@ -558,6 +558,13 @@ func (r *hostRunner) readStepOutput(workDir, stepID string) string {
|
||||
|
||||
// renderRefs 运行期解析残留的 {{steps.X.output}} 占位符(宿主步实参/正文;
|
||||
// workDir 为本步子工作区,经其上两级回到运行根再定位目标步产出)。
|
||||
// 信任不变式(本函数不校验任何东西,只依赖调用顺序):直接调用方只有 doTool
|
||||
// 与 doArtifact,二者所在的宿主步在 Run 里以 strict=true 先跑
|
||||
// verifyPriorWorkflowOutputs,逐个 prompt 步调用 VerifyProjectStepOutput
|
||||
// 重校验来源之后才会走到这里;strict 覆盖 wf.tool/wf.artifact/wf.script/
|
||||
// wf.critic,即所有把上文送模型或落盘的目的步。因此 renderRefs 不会读到
|
||||
// 未重校验的机器产出。新增不以 strict 运行的调用方(如 wf.card/wf.hitl
|
||||
// 直读他步产出)会打破该不变式,必须在新增调用点自行校验,不得依赖本函数。
|
||||
func (r *hostRunner) renderRefs(s, workDir string) string {
|
||||
if !strings.Contains(s, "{{steps.") {
|
||||
return s
|
||||
|
||||
@@ -100,6 +100,17 @@ type ProjectContextSourceRecheckResult struct{ Bundle ProjectContextBundle }
|
||||
|
||||
// ProjectContextSourceRechecker is an optional method of the same registered
|
||||
// ProjectContextProvider. Project workflow reuse fails closed without it.
|
||||
//
|
||||
// Fail-closed comparison contract: Backend compares the returned Bundle with
|
||||
// the recorded receipt as a set of stable identities, never as provider-ordered
|
||||
// output and never by the provider's own echo. Every recorded Included entry
|
||||
// must be answered by exactly one item with the same SourceID, Revision and
|
||||
// Hash whose text still hashes to the recorded InputSHA256; every recorded
|
||||
// OfferedReference and ReadEvidence must be answered by the same
|
||||
// (ID, SourceID, Revision, Hash); Missing must be equal as a set, so a benign
|
||||
// reordering is accepted while an added, dropped or replaced entry is refused.
|
||||
// Anything unproven, extra or absent rejects; reordering never widens
|
||||
// authorization.
|
||||
type ProjectContextSourceRechecker interface {
|
||||
Recheck(context.Context, ProjectContextSourceRecheckRequest) (ProjectContextSourceRecheckResult, error)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user