diff --git a/internal/check/paths.go b/internal/check/paths.go index 3658b69..cc9b295 100644 --- a/internal/check/paths.go +++ b/internal/check/paths.go @@ -63,3 +63,54 @@ func LinkTargetsWithoutADirectory(text string) []string { } return named } + +// LinkTargetsAboveTheirDocument returns the markdown link targets a text +// carries that step out of the document's own directory, in the order it names +// them, each once. A caller joins each one to the directory its document sits +// in and gets a repository-relative path. +// +// This is a third reading rather than either of the two above growing, and the +// reason is that both of them are already exact about a different population. +// PathsNamedInProse requires a leading segment naming a directory of this tree, +// and `..` is not one; widening it would make it read a pair of full stops in a +// sentence. LinkTargetsWithoutADirectory skips a target carrying a slash, which +// is what its name says it returns and what its own comment hands to the prose +// pattern. A target spelled `../NAME` falls between the two: it is inside +// parentheses, so the intent is there, and it carries a slash, so the reading +// that resolves intent will not touch it. +// +// The shape it is for is a document under docs/ naming a file at the root, +// which is the only way one can write that link. `docs/operator-guide.md` +// pointing at `../LICENSE` is a reader being told a file is there, and until +// this existed nothing in this tree read that link at all. +// +// WHAT THE CALLER STILL OWES. The target is relative to the document and this +// returns it as written, so a caller that resolves it against the root instead +// resolves the wrong thing. It also has to decide what to do with a target that +// climbs past the root of the tree, because this returns that one too: `../..` +// from a document one directory down names something outside the checkout, and +// whether such a file exists is not a question any reading of this tree +// answers. +func LinkTargetsAboveTheirDocument(text string) []string { + var named []string + seen := make(map[string]bool) + + for _, match := range linkTarget.FindAllStringSubmatch(text, -1) { + target := match[1] + if cut := strings.IndexAny(target, "#?"); cut >= 0 { + target = target[:cut] + } + if !strings.HasPrefix(target, "../") { + continue + } + // A scheme or a backslash is left alone for the same reasons the + // reading above leaves them alone: one is somebody else's server + // and the other is not how a path is written here. + if strings.ContainsAny(target, ":\\") || seen[target] { + continue + } + seen[target] = true + named = append(named, target) + } + return named +} diff --git a/internal/check/paths_test.go b/internal/check/paths_test.go index 9afef0c..aee6d6d 100644 --- a/internal/check/paths_test.go +++ b/internal/check/paths_test.go @@ -87,3 +87,83 @@ func TestWhatCountsAsALinkToAFileBesideTheDocument(t *testing.T) { }) } } + +// TestWhatCountsAsALinkToAFileAboveTheDocument exercises the third reader +// directly, for the same reason the one above is exercised directly. A fixture +// is a tree and a verdict, so it proves that a document under docs/ linking a +// root file that is not there is refused and one linking a file that is there +// is not, and it says nothing about a target carrying a scheme or a fragment, +// about a target repeated, or about the order two of them come back in. Those +// arms all fail as silence, which is what a passing suite looks like. +func TestWhatCountsAsALinkToAFileAboveTheDocument(t *testing.T) { + tests := []struct { + name string + text string + want []string + }{ + { + name: "a root file linked from one directory down", + text: "The terms are in [LICENSE](../LICENSE) at the root.", + want: []string{"../LICENSE"}, + }, + { + name: "a fragment on the end", + text: "See [the notice](../NOTICE.md#scope).", + want: []string{"../NOTICE.md"}, + }, + { + name: "the same target twice", + text: "[one](../LICENSE) and [again](../LICENSE)", + want: []string{"../LICENSE"}, + }, + { + name: "two targets keep the order they were written in", + text: "[first](../NOTICE.md) then [second](../LICENSE)", + want: []string{"../NOTICE.md", "../LICENSE"}, + }, + { + name: "a climb of more than one directory is returned for the caller to place", + text: "See [it](../../NOTICE.md).", + want: []string{"../../NOTICE.md"}, + }, + { + name: "a directory on the way back down", + text: "See [the record](../docs/decisions/0002-repository-layout.md).", + want: []string{"../docs/decisions/0002-repository-layout.md"}, + }, + + { + name: "a target beside the document is the second reader's", + text: "See [the notice](NOTICE.md) beside this page.", + }, + { + name: "a target under a directory is the prose pattern's", + text: "See [the record](docs/decisions/0002-repository-layout.md).", + }, + { + name: "two full stops in a sentence rather than in a link", + text: "The file is one directory up, at ../LICENSE, and this names it.", + }, + { + name: "a target on somebody else's server that climbs", + text: "See [the page](https://example.invalid/../NOTICE.md).", + }, + { + name: "a windows separator after the climb", + text: `See [the record](..\notes.md).`, + }, + { + name: "a place inside the same page", + text: "See [further down](#what-was-run).", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := LinkTargetsAboveTheirDocument(tc.text) + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Errorf("read %v, want %v", got, tc.want) + } + }) + } +} diff --git a/internal/invariants/invariants.go b/internal/invariants/invariants.go index 5d77904..3d29d1f 100644 --- a/internal/invariants/invariants.go +++ b/internal/invariants/invariants.go @@ -497,7 +497,7 @@ func pathsLeg(root string, texts []textFile) (Leg, []Refusal) { } // pathsNamedIn returns every repository-relative path one document names, by -// both readings, each once. The two readings are joined here rather than +// all three readings, each once. The readings are joined here rather than // refused separately so that one property is produced at one site: a document // that writes a path in a sentence and links it as well is one dead pointer and // is worth one refusal. @@ -506,6 +506,13 @@ func pathsLeg(root string, texts []textFile) (Leg, []Refusal) { // join uses the document's own directory and not the root. docs/privacy.md // links supply-chain.md and means docs/supply-chain.md; resolving that against // the root would refuse the file that is there and pass the one that is not. +// +// The third reading is the one that reaches a document under docs/ naming a +// file at the root, which is written ../NAME and is the only way it can be +// written. That target carries a slash, so the reading above hands it on, and +// it opens with two full stops, so the prose pattern never had it. It was the +// shape both of them sat either side of, and until it was read a link nobody +// could follow was the one that passed. func pathsNamedIn(document textFile) []string { named := check.PathsNamedInProse(document.text) @@ -526,9 +533,34 @@ func pathsNamedIn(document textFile) []string { seen[beside] = true named = append(named, beside) } + + for _, target := range check.LinkTargetsAboveTheirDocument(document.text) { + above := path.Join(directory, target) + if outsideTheTree(above) { + // The target climbs past the root of the checkout. Whether a + // file out there exists is not a question this tree answers, + // and joining it to the root anyway would ask the filesystem + // about a path this repository does not hold. It is left alone + // for the same reason a target with a scheme in it is. + continue + } + if seen[above] { + continue + } + seen[above] = true + named = append(named, above) + } return named } +// outsideTheTree says whether a path that has already been cleaned still points +// above the root it was resolved from. path.Join collapses what it can, so what +// is left leading with a pair of full stops is what could not be collapsed, and +// a target that resolves to the root itself names no file either. +func outsideTheTree(cleaned string) bool { + return cleaned == "." || cleaned == ".." || strings.HasPrefix(cleaned, "../") +} + // isOwnDocument says whether a path is one of this repository's own documents. // A file at the root is one; anything under docs/ is one; nothing else is, // because a path named in a comment inside the runner is held by the compiler diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected new file mode 100644 index 0000000..733c34f --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected @@ -0,0 +1 @@ +text-files 3 diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected-refusals b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected-refusals new file mode 100644 index 0000000..159931d --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/expected-refusals @@ -0,0 +1 @@ +document-names-a-path-that-does-not-resolve diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/near-neighbour b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/near-neighbour new file mode 100644 index 0000000..18191be --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/near-neighbour @@ -0,0 +1 @@ +a-document-under-docs-linking-a-root-file-that-is-there diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/NOTICE.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/NOTICE.md new file mode 100644 index 0000000..e3cfa99 --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/NOTICE.md @@ -0,0 +1,5 @@ +# Notice + +This software is developed for lawful use. Operators and users are responsible +for making sure that their deployment and use comply with the laws that apply +to them. diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/README.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/README.md new file mode 100644 index 0000000..547bbec --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/README.md @@ -0,0 +1,4 @@ +# a tree + +See [NOTICE.md](NOTICE.md) for the intended-use notice. The guide is in +[docs/guide.md](docs/guide.md). diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/docs/guide.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/docs/guide.md new file mode 100644 index 0000000..21832ea --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-not-there/tree/docs/guide.md @@ -0,0 +1,4 @@ +# The guide + +The terms this tree is published under are in [LICENSE](../LICENSE) at the root +of the checkout, one directory above this page. diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/expected b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/expected new file mode 100644 index 0000000..3d7cc38 --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/expected @@ -0,0 +1 @@ +text-files 4 diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/expected-refusals b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/expected-refusals new file mode 100644 index 0000000..e69de29 diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/LICENSE b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/LICENSE new file mode 100644 index 0000000..ba94d74 --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/LICENSE @@ -0,0 +1 @@ +The terms this tree is published under. diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/NOTICE.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/NOTICE.md new file mode 100644 index 0000000..e3cfa99 --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/NOTICE.md @@ -0,0 +1,5 @@ +# Notice + +This software is developed for lawful use. Operators and users are responsible +for making sure that their deployment and use comply with the laws that apply +to them. diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/README.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/README.md new file mode 100644 index 0000000..547bbec --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/README.md @@ -0,0 +1,4 @@ +# a tree + +See [NOTICE.md](NOTICE.md) for the intended-use notice. The guide is in +[docs/guide.md](docs/guide.md). diff --git a/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/docs/guide.md b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/docs/guide.md new file mode 100644 index 0000000..21832ea --- /dev/null +++ b/testdata/invariants/a-document-under-docs-linking-a-root-file-that-is-there/tree/docs/guide.md @@ -0,0 +1,4 @@ +# The guide + +The terms this tree is published under are in [LICENSE](../LICENSE) at the root +of the checkout, one directory above this page.