amadan

carlos / platform Public

copy gate: server text is never a package-level value, and its maker never writes a literal or a constant (Task 1a fix round 3)

The re-review's two remaining holes in server text's provenance, closed as a
drift lint (the T15 scope ruling), no further:

1. checkServerText fails a package-level var or const whose type is or
   holds serverText, or whose value is a composite literal of it.
2. Inside mover.roundTrip, a serverText's field may not be a string literal
   or a named constant (package or local); it is a value read from the
   decoded response.

Fixtures testdata/moves-copy/sinks/server-text-var and server-text-maker,
held by TestMovesServerTextRefusesForgedValues.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 08936499d71128253b6a61483bdbc3c758c041b4 parent 4db9fe5
4 files changed, +158 −1
  • approved_moves_copy_scan_test.go +78 −1
  • approved_moves_copy_test.go +35 −0
  • testdata/moves-copy/sinks/server-text-maker/emit.go +29 −0
  • testdata/moves-copy/sinks/server-text-var/emit.go +16 −0
diff --git a/approved_moves_copy_scan_test.go b/approved_moves_copy_scan_test.go
index b3d8149cf..3139e1279 100644
--- a/approved_moves_copy_scan_test.go
+++ b/approved_moves_copy_scan_test.go
@@ -2052,6 +2052,8 @@ func checkServerText(t *testing.T, file string, spec movesServerTextSpec) []serv
}
var out []serverTextProblem
maker, sink := false, false
+ var files []*ast.File
+ consts := map[string]bool{} // the package's named constants
for _, name := range names {
if strings.HasSuffix(name, "_test.go") {
continue
@@ -2060,7 +2062,42 @@ func checkServerText(t *testing.T, file string, spec movesServerTextSpec) []serv
if err != nil {
t.Fatal(err)
}
+ files = append(files, f)
for _, d := range f.Decls {
+ if g, ok := d.(*ast.GenDecl); ok && g.Tok == token.CONST {
+ for _, sp := range g.Specs {
+ for _, n := range sp.(*ast.ValueSpec).Names {
+ consts[n.Name] = true
+ }
+ }
+ }
+ }
+ }
+ mentions := func(n ast.Node) bool {
+ found := false
+ ast.Inspect(n, func(x ast.Node) bool {
+ if id, ok := x.(*ast.Ident); ok && id.Name == spec.Type {
+ found = true
+ }
+ return !found
+ })
+ return found
+ }
+ for _, f := range files {
+ for _, d := range f.Decls {
+ // Server text is never a package-level value: a var or const whose
+ // type is or holds the type, or a composite literal of it outside
+ // a function body (fix round 3, hole 1).
+ if g, ok := d.(*ast.GenDecl); ok && (g.Tok == token.VAR || g.Tok == token.CONST) {
+ for _, sp := range g.Specs {
+ if vs := sp.(*ast.ValueSpec); mentions(vs) {
+ for _, n := range vs.Names {
+ out = append(out, serverTextProblem{"<package>", fmt.Sprintf("declares %s, a package-level %s", n.Name, spec.Type)})
+ }
+ }
+ }
+ continue
+ }
fd, ok := d.(*ast.FuncDecl)
if !ok || fd.Body == nil {
continue
@@ -2071,6 +2108,17 @@ func checkServerText(t *testing.T, file string, spec movesServerTextSpec) []serv
}
writes := map[ast.Node]bool{}
readsBody, callsDo := false, false
+ localConsts := map[string]bool{}
+ ast.Inspect(fd.Body, func(n ast.Node) bool {
+ if g, ok := n.(*ast.GenDecl); ok && g.Tok == token.CONST {
+ for _, sp := range g.Specs {
+ for _, nm := range sp.(*ast.ValueSpec).Names {
+ localConsts[nm.Name] = true
+ }
+ }
+ }
+ return true
+ })
ast.Inspect(fd.Body, func(n ast.Node) bool {
switch n := n.(type) {
case *ast.AssignStmt:
@@ -2083,8 +2131,37 @@ func checkServerText(t *testing.T, file string, spec movesServerTextSpec) []serv
}
}
case *ast.CompositeLit:
- if id, ok := n.Type.(*ast.Ident); ok && id.Name == spec.Type && fn != spec.Maker {
+ id, ok := n.Type.(*ast.Ident)
+ if !ok || id.Name != spec.Type {
+ break
+ }
+ if fn != spec.Maker {
out = append(out, serverTextProblem{fd.Name.Name, fmt.Sprintf("makes a %s outside %s", spec.Type, spec.Maker)})
+ break
+ }
+ // In the maker the text is a value read from the decoded
+ // response: never a string literal or a named constant
+ // (fix round 3, hole 2).
+ for _, el := range n.Elts {
+ v := el
+ if kv, ok := el.(*ast.KeyValueExpr); ok {
+ v = kv.Value
+ }
+ for p, ok := v.(*ast.ParenExpr); ok; p, ok = v.(*ast.ParenExpr) {
+ v = p.X
+ }
+ what := ""
+ switch v := v.(type) {
+ case *ast.BasicLit:
+ what = v.Value
+ case *ast.Ident:
+ if consts[v.Name] || localConsts[v.Name] {
+ what = v.Name
+ }
+ }
+ if what != "" {
+ out = append(out, serverTextProblem{fd.Name.Name, fmt.Sprintf("makes a %s from %s, not a value read from the response", spec.Type, what)})
+ }
}
case *ast.SelectorExpr:
switch {
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index 8cad37a64..0f8e275ac 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -1735,3 +1735,38 @@ func TestMovesServerTextHasOneMaker(t *testing.T) {
t.Fatal("move.go declares no serverText")
}
}
+
+// Fix round 3 of Plan 6 Task 1a: two cheap holes in server text's provenance.
+// Server text may not be a package-level value (a var or const of a type that
+// is or holds serverText, or a composite literal outside a function body), and
+// inside its one maker its field is a value read from the decoded response,
+// never a string literal or a named constant.
+func TestMovesServerTextRefusesForgedValues(t *testing.T) {
+ spec := movesServerTextSpec{Type: "serverText", Field: "spoken", Maker: "mover.roundTrip", Sink: "saySpoken"}
+ for _, c := range []struct {
+ dir string
+ want []string
+ }{
+ {"server-text-var", []string{
+ "<package>: declares forged, a package-level serverText",
+ "<package>: declares forgedList, a package-level serverText",
+ }},
+ {"server-text-maker", []string{
+ `roundTrip: makes a serverText from "unreviewed literal in the maker", not a value read from the response`,
+ "roundTrip: makes a serverText from unreviewed, not a value read from the response",
+ "roundTrip: makes a serverText from local, not a value read from the response",
+ }},
+ } {
+ file := filepath.Join("testdata", "moves-copy", "sinks", c.dir, "emit.go")
+ var got []string
+ for _, p := range checkServerText(t, file, spec) {
+ got = append(got, p.String())
+ }
+ sort.Strings(got)
+ want := append([]string(nil), c.want...)
+ sort.Strings(want)
+ if strings.Join(got, "\n") != strings.Join(want, "\n") {
+ t.Errorf("%s:\n got %q\nwant %q", c.dir, got, want)
+ }
+ }
+}
diff --git a/testdata/moves-copy/sinks/server-text-maker/emit.go b/testdata/moves-copy/sinks/server-text-maker/emit.go
new file mode 100644
index 000000000..6a7cf0f97
--- /dev/null
+++ b/testdata/moves-copy/sinks/server-text-maker/emit.go
@@ -0,0 +1,29 @@
+package fixture
+
+// Not compiled. The one maker of server text, writing a literal or a named
+// constant instead of a value read from the decoded response (fix round 3 of
+// Plan 6 Task 1a, hole 2). Only the last return is legitimate.
+
+type serverText struct{ spoken string }
+
+type mover struct{ httpc client }
+
+const unreviewed = "unreviewed constant"
+
+func saySpoken(out int, t serverText) {}
+
+func (m *mover) roundTrip(kind int) serverText {
+ res, _ := m.httpc.Do(nil)
+ var e struct{ Error string }
+ _ = json.NewDecoder(res.Body).Decode(&e)
+ switch kind {
+ case 0:
+ return serverText{spoken: "unreviewed literal in the maker"}
+ case 1:
+ return serverText{spoken: unreviewed}
+ case 2:
+ const local = "unreviewed local constant"
+ return serverText{local}
+ }
+ return serverText{spoken: e.Error}
+}
diff --git a/testdata/moves-copy/sinks/server-text-var/emit.go b/testdata/moves-copy/sinks/server-text-var/emit.go
new file mode 100644
index 000000000..0bb40cc58
--- /dev/null
+++ b/testdata/moves-copy/sinks/server-text-var/emit.go
@@ -0,0 +1,16 @@
+package fixture
+
+// Not compiled. Server text forged at package level, outside any function
+// body, then said through its sink (fix round 3 of Plan 6 Task 1a, hole 1).
+
+type serverText struct{ spoken string }
+
+var forged = serverText{spoken: "unreviewed package var"}
+
+var forgedList = []serverText{{spoken: "unreviewed list element"}}
+
+func saySpoken(out int, t serverText) {}
+
+func say(out int) {
+ saySpoken(out, forged)
+}