Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Phase 7 slice 1: review on the web, so approving does not start with a clone

78912785b00227cffe3f8fb26b057cd60159a6b4 · Keenan Nemetz · 18d ago · parent 9bea7f11

Phase 7 slice 1: review on the web, so approving does not start with a clone

Phase 7 is re-scoped: signing is deferred (the SSH handshake already establishes
who pushed, and there is no third-party verifier here), and the phase is now about
the experience of using the forge.

- GET /r/<repo>/commit/<sha>: a commit's metadata and the patch it introduced,
  per-file hunks with line numbers; the commit list links to it
- change requests show the same diff, computed three-dot: a review is what the
  branch adds since it diverged, not every commit main gained meanwhile
- diffs are capped at 512 KiB and labelled when cut, binary files are named
  rather than faked, and every line is escaped (a <script> line is a test)
- the repository README renders on its home page and markdown blobs render as
  markdown, through the sanitiser issues already use

internal/server/web_test.go +172

@@ -0,0 +1,172 @@
1 package server_test
2
3 // The read-only web has to be enough to review a change: a commit page with the
4 // patch it introduced, and a change request that shows what would land.
5
6 import (
7 "os"
8 "path/filepath"
9 "strings"
10 "testing"
11 )
12
13 func TestCommitPageShowsThePatch(t *testing.T) {
14 f := newFixture(t)
15 f.publish(f.repo)
16
17 work := workRepo(t, map[string]string{"readme.md": "v1 line\nkeep me\n"})
18 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
19 t.Fatalf("initial push: %s", errs)
20 }
21 if err := os.WriteFile(filepath.Join(work, "readme.md"), []byte("v2 line\nkeep me\nbrand new\n"), 0o644); err != nil {
22 t.Fatal(err)
23 }
24 if _, errs, err := runGit(t, work, "add", "-A"); err != nil {
25 t.Fatalf("add: %s", errs)
26 }
27 if _, errs, err := runGit(t, work, "commit", "-qm", "change the readme"); err != nil {
28 t.Fatalf("commit: %s", errs)
29 }
30 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
31 t.Fatalf("second push: %s", errs)
32 }
33 sha := mustSHA(t, work)
34
35 code, body := f.get("/r/" + f.repo + "/commit/" + sha)
36 if code != 200 {
37 t.Fatalf("commit page = %d", code)
38 }
39 for _, want := range []string{"change the readme", "readme.md", "v2 line", "v1 line", "brand new"} {
40 if !strings.Contains(body, want) {
41 t.Fatalf("commit page does not show %q:\n%s", want, body)
42 }
43 }
44 // Added and removed lines are marked as such, not just printed: the colour
45 // and the numbering both hang off these classes.
46 for _, want := range []string{`class="add"`, `class="del"`} {
47 if !strings.Contains(body, want) {
48 t.Fatalf("commit page has no %s row:\n%s", want, body)
49 }
50 }
51 // The subject links back from the commit list, so the page is reachable by
52 // clicking rather than by knowing a sha.
53 if code, body := f.get("/r/" + f.repo + "/commits/main"); code != 200 || !strings.Contains(body, "/commit/"+sha) {
54 t.Fatalf("commits page does not link to the commit (%d)", code)
55 }
56 // A sha that does not exist is a 404, not a 500.
57 if code, _ := f.get("/r/" + f.repo + "/commit/deadbeefdeadbeef"); code != 404 {
58 t.Fatalf("unknown commit = %d, want 404", code)
59 }
60 }
61
62 func TestRequestPageShowsWhatWouldLand(t *testing.T) {
63 f := newFixture(t)
64 f.publish(f.repo)
65
66 base := workRepo(t, map[string]string{"readme.md": "v1\n"})
67 if _, errs, err := f.git(base, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
68 t.Fatalf("main push: %s", errs)
69 }
70
71 feature, head := f.cloneWithChange(t, map[string]string{"readme.md": "v1\nfeature line\n"}, "feature")
72 if _, errs, err := f.git(feature, "push", f.sshURL(f.repo), "HEAD:refs/heads/feature"); err != nil {
73 t.Fatalf("feature push: %s", errs)
74 }
75
76 meta := f.newMetaRepo(t)
77 if err := meta.apply(map[string]string{
78 "requests/" + reqID + "/meta.json": requestMeta("main", head, "keenan", f.ownerID),
79 "requests/" + reqID + "/description.md": "why this change\n",
80 }, "request"); err != nil {
81 t.Fatalf("request push: %v", err)
82 }
83
84 code, body := f.get("/r/" + f.repo + "/requests/" + reqID)
85 if code != 200 {
86 t.Fatalf("request page = %d", code)
87 }
88 for _, want := range []string{"Land the feature", "readme.md", "feature line", "why this change", `class="add"`} {
89 if !strings.Contains(body, want) {
90 t.Fatalf("request page does not show %q:\n%s", want, body)
91 }
92 }
93 // The request page is the approval surface, so it must not need a clone.
94 if strings.Contains(body, "No changes, or the head is not reachable") {
95 t.Fatalf("request page found no diff for a branch with a change:\n%s", body)
96 }
97 }
98
99 // TestCommitPageEscapesFileContent matters because the web renders other people's
100 // commits: a diff that executes a tag is an XSS, not a syntax error.
101 func TestCommitPageEscapesFileContent(t *testing.T) {
102 f := newFixture(t)
103 f.publish(f.repo)
104 work := workRepo(t, map[string]string{"evil.html": "safe\n"})
105 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
106 t.Fatalf("first push: %s", errs)
107 }
108 if err := os.WriteFile(filepath.Join(work, "evil.html"), []byte("<script>alert(1)</script>\n"), 0o644); err != nil {
109 t.Fatal(err)
110 }
111 if _, errs, err := runGit(t, work, "add", "-A"); err != nil {
112 t.Fatal(errs)
113 }
114 if _, errs, err := runGit(t, work, "commit", "-qm", "add a tag"); err != nil {
115 t.Fatal(errs)
116 }
117 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
118 t.Fatalf("push: %s", errs)
119 }
120 _, body := f.get("/r/" + f.repo + "/commit/" + mustSHA(t, work))
121 if strings.Contains(body, "<script>alert(1)</script>") {
122 t.Fatalf("the diff put live markup into the page:\n%s", body)
123 }
124 if !strings.Contains(body, "&lt;script&gt;alert(1)&lt;/script&gt;") {
125 t.Fatalf("the diff does not show the line escaped:\n%s", body)
126 }
127 }
128
129 // TestReadmeIsTheRepoFrontPage: a forge that shows a bare file list is a
130 // filesystem, not a project page.
131 func TestReadmeIsTheRepoFrontPage(t *testing.T) {
132 f := newFixture(t)
133 f.publish(f.repo)
134 work := workRepo(t, map[string]string{
135 "README.md": "# Demo project\n\nHow to run it.\n",
136 "main.go": "package main\n",
137 })
138 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
139 t.Fatalf("push: %s", errs)
140 }
141 code, body := f.get("/r/" + f.repo)
142 if code != 200 {
143 t.Fatalf("repo page = %d", code)
144 }
145 if !strings.Contains(body, "<h1>Demo project</h1>") || !strings.Contains(body, "How to run it.") {
146 t.Fatalf("README was not rendered on the repo page:\n%s", body)
147 }
148 // Markdown blobs render too, while source files stay source.
149 if code, body := f.get("/r/" + f.repo + "/blob/main/README.md"); code != 200 || !strings.Contains(body, "<h1>Demo project</h1>") {
150 t.Fatalf("markdown blob = %d:\n%s", code, body)
151 }
152 if code, body := f.get("/r/" + f.repo + "/blob/main/main.go"); code != 200 || !strings.Contains(body, "package main") {
153 t.Fatalf("source blob = %d", code)
154 }
155 }
156
157 // TestReadmeIsSanitised: a README is pushed content, so it crosses the same
158 // boundary as an issue body.
159 func TestReadmeIsSanitised(t *testing.T) {
160 f := newFixture(t)
161 f.publish(f.repo)
162 work := workRepo(t, map[string]string{
163 "README.md": "# hi\n\n<script>alert(1)</script>\n",
164 })
165 if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
166 t.Fatalf("push: %s", errs)
167 }
168 _, body := f.get("/r/" + f.repo)
169 if strings.Contains(body, "<script>alert(1)</script>") {
170 t.Fatalf("rendered README executed a tag:\n%s", body)
171 }
172 }