From 00f5986350f61d7d5fbf48231d6125c8aeab339a Mon Sep 17 00:00:00 2001
From: Will Norris <will@tailscale.com>
Date: Fri, 25 Apr 2025 09:59:14 -0700
Subject: [PATCH] disallow accidental updates to existing links

Prior to #177, our XSRF tokens were bound to link IDs, with a special
`.new` value used for newly created links. So if a user tried to create
a link that already existed, the XSRF check would fail. After #177, this
now silently allows the user to overwrite the existing link without any
indication that this happened.

This change adds a hidden `update` param to the details edit form that
must be present when updating an existing link.

Updates #177

Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d
Signed-off-by: Will Norris <will@tailscale.com>
---
 golink.go        |  7 +++++++
 golink_test.go   | 25 +++++++++++++++++++++++--
 tmpl/detail.html |  1 +
 tmpl/help.html   |  3 +++
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/golink.go b/golink.go
index cf8b943..74bcc10 100644
--- a/golink.go
+++ b/golink.go
@@ -881,6 +881,13 @@ func serveSave(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Prevent accidental overwrites of existing links.
+	// If the link already exists, make sure this request is an intentional update.
+	if link != nil && r.FormValue("update") == "" {
+		http.Error(w, "link already exists", http.StatusForbidden)
+		return
+	}
+
 	if !canEditLink(r.Context(), link, cu) {
 		http.Error(w, fmt.Sprintf("cannot update link owned by %q", link.Owner), http.StatusForbidden)
 		return
diff --git a/golink_test.go b/golink_test.go
index b71c752..8d6f1f5 100644
--- a/golink_test.go
+++ b/golink_test.go
@@ -230,6 +230,7 @@ func TestServeSave(t *testing.T) {
 		name              string
 		short             string
 		long              string
+		update            bool
 		allowUnknownUsers bool
 		currentUser       func(*http.Request) (user, error)
 		wantStatus        int
@@ -252,6 +253,19 @@ func TestServeSave(t *testing.T) {
 			long:       "http://who/",
 			wantStatus: http.StatusOK,
 		},
+		{
+			name:       "disallow accidental updates",
+			short:      "who",
+			long:       "http://who2/",
+			wantStatus: http.StatusForbidden,
+		},
+		{
+			name:       "allow intentional updates",
+			short:      "who",
+			long:       "http://who/",
+			update:     true,
+			wantStatus: http.StatusOK,
+		},
 		{
 			name:        "disallow editing another's link",
 			short:       "who",
@@ -263,6 +277,7 @@ func TestServeSave(t *testing.T) {
 			name:        "allow editing link owned by tagged-devices",
 			short:       "link-owned-by-tagged-devices",
 			long:        "/after",
+			update:      true,
 			currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com"}, nil },
 			wantStatus:  http.StatusOK,
 		},
@@ -270,6 +285,7 @@ func TestServeSave(t *testing.T) {
 			name:        "admins can edit any link",
 			short:       "who",
 			long:        "http://who/",
+			update:      true,
 			currentUser: func(*http.Request) (user, error) { return user{login: "bar@example.com", isAdmin: true}, nil },
 			wantStatus:  http.StatusOK,
 		},
@@ -304,10 +320,15 @@ func TestServeSave(t *testing.T) {
 			*allowUnknownUsers = tt.allowUnknownUsers
 			t.Cleanup(func() { *allowUnknownUsers = oldAllowUnknownUsers })
 
-			r := httptest.NewRequest("POST", "/", strings.NewReader(url.Values{
+			v := url.Values{
 				"short": {tt.short},
 				"long":  {tt.long},
-			}.Encode()))
+			}
+			if tt.update {
+				v.Set("update", "1")
+			}
+
+			r := httptest.NewRequest("POST", "/", strings.NewReader(v.Encode()))
 			r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 			w := httptest.NewRecorder()
 			serveSave(w, r)
diff --git a/tmpl/detail.html b/tmpl/detail.html
index f7c3ccb..744807a 100644
--- a/tmpl/detail.html
+++ b/tmpl/detail.html
@@ -26,6 +26,7 @@ <h2 class="text-xl font-bold pb-2">Link Details</h2>
         <dd>{{.Link.LastEdit.Format "Jan _2, 2006 3:04pm MST"}}</dd>
       </dl>
 
+      <input type="hidden" name="update" value="1">
       <button type=submit class="py-2 px-4 my-4 rounded-md bg-blue-500 border-blue-500 text-white hover:bg-blue-600 hover:border-blue-600">Update</button>
     </form>
 
diff --git a/tmpl/help.html b/tmpl/help.html
index 7b66b61..ea4a998 100644
--- a/tmpl/help.html
+++ b/tmpl/help.html
@@ -139,5 +139,8 @@ <h2 id="api">Application Programming Interface (API)</h2>
 {{`{"Short":"cs","Long":"https://cs.github.com/","Created":"2022-06-03T22:15:29.993978392Z","LastEdit":"2022-06-03T22:15:29.993978392Z","Owner":"amelie@example.com"}`}}
 </pre>
 
+<p>
+To update an existing link, also include `-d update=1`.
+
 </article>
 {{ end }}