Release corrections round 3: redact candidate-lookup failures in logs (#104)

Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>

internal/server/dinners.go

@@ -205,7 +205,8 @@ func actorOrFail(w http.ResponseWriter, r *http.Request, logger *slog.Logger) (a
 // an archived dinner is 409 dinner_archived and one changed while grounding
 // 409 ground_conflict. Any other grounding failure, by candidate, link or
 // manual recipe, wraps dinners.ErrGroundFailed and is 500, logged by reason
-// and dinner id only.
+// and dinner id only, as is any other failure finding candidates, which
+// wraps dinners.ErrCandidatesFailed.
 func writeDinnerError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) {
 	var invalid *dinners.ValidationError
 	var dup *dinners.DuplicateError
@@ -231,6 +232,12 @@ func writeDinnerError(w http.ResponseWriter, r *http.Request, logger *slog.Logge
 		// logged.
 		logger.ErrorContext(r.Context(), action, "reason", "ground_failed", "dinner_id", chi.URLParam(r, "id"))
 		writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error")
+	case errors.Is(err, dinners.ErrCandidatesFailed):
+		// The error may quote a candidate's URL, which can carry a
+		// personal token, and dependency errors, so only the reason and
+		// dinner are logged.
+		logger.ErrorContext(r.Context(), action, "reason", "candidates_failed", "dinner_id", chi.URLParam(r, "id"))
+		writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error")
 	case errors.Is(err, dinners.ErrArchived):
 		writeError(w, r, logger, http.StatusConflict, "dinner_archived", "the dinner is archived")
 	case errors.Is(err, dinners.ErrGroundConflict):

internal/server/grounding_test.go

@@ -204,6 +204,57 @@ func TestGetRecipeCandidatesSearchFails(t *testing.T) {
 	}
 }
 
+// TestGetRecipeCandidatesStoredLookupFailure: a candidate whose stored
+// recipe cannot be read, or is stored incomplete, is a 500 logged by reason
+// and dinner id, never with the candidate's URL, which can carry a personal
+// token, or the database error.
+func TestGetRecipeCandidatesStoredLookupFailure(t *testing.T) {
+	t.Parallel()
+	const (
+		token = "personal-token-abc123"
+		host  = "secret-candidate.example"
+		page  = "https://" + host + "/curry?access_token=" + token
+	)
+	for _, tc := range []struct {
+		name, sabotage string
+		leaks          []string
+	}{
+		// Renaming a column the lookup selects fails the read.
+		{"lookup", `ALTER TABLE recipes RENAME COLUMN yield TO yield_gone`, []string{"no such column", "yield"}},
+		// A recipe stored without its photo, method or difficulty is
+		// incomplete.
+		{"incomplete", `INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
+			VALUES ('r-incomplete', '` + page + `', 'Curry', '{}', 'now', 'now', 'now')`, []string{"r-incomplete", "no photo"}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			e := newDinnersEnv(t)
+			e.search.Results["Chicken curry"] = []recipes.Candidate{{URL: page}}
+			id := createDinner(t, e.h, `{"name":"Chicken curry"}`) // runner not started: stays pending
+			if _, err := e.st.Writer.ExecContext(t.Context(), tc.sabotage); err != nil {
+				t.Fatal(err)
+			}
+
+			rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
+			if rec.Code != http.StatusInternalServerError || errorCode(body) != "internal" {
+				t.Errorf("GET = %d %s, want 500 internal", rec.Code, rec.Body)
+			}
+			logs := e.logs.String()
+			for _, secret := range append([]string{page, host, token, "access_token"}, tc.leaks...) {
+				if strings.Contains(rec.Body.String(), secret) {
+					t.Errorf("response contains %q: %s", secret, rec.Body)
+				}
+				if strings.Contains(logs, secret) {
+					t.Errorf("logs contain %q: %s", secret, logs)
+				}
+			}
+			if !strings.Contains(logs, "reason=candidates_failed") || !strings.Contains(logs, "dinner_id="+id) {
+				t.Errorf("logs = %q, want reason=candidates_failed and the dinner id", logs)
+			}
+		})
+	}
+}
+
 func TestPostGroundViaCandidate(t *testing.T) {
 	t.Parallel()
 	e := newDinnersEnv(t).withRecipes(t)