fix: survive broken D2L topics, honest body-state reporting, URL escaping, credential-leak fix

CHANGELOG.md

@@ -315,6 +315,97 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   `html.UnescapeString`, so the default text body decodes common
   named and numeric entities (`&`, `<`, `'`, `'`, …)
   rather than leaving them as raw HTML escape sequences.
+- **P1:** `archive update` aborted the entire pass on the first topic
+  whose metadata fetch failed. D2L keeps stale "broken" TOC entries
+  (`IsBroken: true`, 404 on the topics API) — one dead 2022 dropbox
+  link in a single course blocked every course queued after it from
+  archiving. Topics D2L flags broken are now skipped without an API
+  call and topic-level fetch failures are counted and retried on the
+  next pass; a 401/403 (or a login-page redirect) stops the run
+  directly with `session expired; run 'schooltools login'`. A burst of
+  consecutive transport failures (network down, DNS dead) now aborts
+  the pass too instead of sending one doomed request per topic and
+  reporting success — HTTP failures such as a 404 on a deleted topic
+  keep the continue behavior. Course-level fetch and storage errors
+  still stop the run. The update summary and `--json` envelope gain
+  `topicsFailed` / `topicsSkipped` counters, and `--dry-run` reports
+  broken topics so the plan matches what a real pass does.
+- **P1:** `archive` status and `archive find` trusted the index's
+  `currentBody` pointer instead of the filesystem, so a store with
+  186 dangling pointers (from the `prune` entry under
+  Unreleased) reported "Bodies: 326 present, 8 missing" while
+  186 blobs were actually gone. `LoadStatus` now stats each blob
+  (a stat error other than "not exist" no longer counts as
+  missing); `find --with-bodies` gained
+  the `WithBodiesOnDisk` filter and `find --missing-bodies` also
+  matches topics whose pointer exists but whose blob does not.
+  `HasBody` on search hits and `FileTopic` reflects on-disk state.
+  `archive export` keeps the old pointer semantics so its
+  "skipped-missing-body" reporting is unchanged.
+- **P2:** D2L file-body URLs that contain unescaped path characters —
+  literal spaces, a bare `%` (`Acetic Acid, 5% (Vinegar).pdf`), `#` in
+  filenames (`Quiz #3 …`), a literal `?` — either failed `url.Parse`
+  with `invalid URL escape` or were truncated at the `#` into a URL
+  fragment and 404'd. The new `internal/urlutil` re-encodes the path
+  portion of a reference (preserving already-valid `%XX` sequences;
+  D2L file URLs never carry a query string, so `?` is a path
+  character) and applies the same cleaning to absolute URLs; both
+  `archive update`'s body downloads and `download` route through it.
+- **P2:** A course pass that hit a fatal error (expired session, or the
+  network going away mid-pass) returned before writing the per-course
+  index, so blobs fetched earlier in that pass were left on disk with
+  nothing pointing at them and the index updates were dropped. The
+  accumulated index is now persisted on the abort paths. A topic whose
+  metadata advanced but whose body could not be fetched now clears its
+  stale `currentBody` pointer (the blob itself is never removed), so the
+  next pass still sees it as needing a body instead of treating the old
+  bytes as current.
+- **P2:** `archive verify` treated any stat failure other than "not
+  exist" as proof a blob was present (a side effect of making the
+  status/search "is this body missing?" check tolerant of permission
+  errors). It now reports a present-but-unstattable blob as
+  `unreadable-*-blob` and no longer reports `ok` for it.
+- **P2:** `systemd status` dropped `systemctl list-timers` data rows
+  whose LAST column was a placeholder (a timer that never fired), so
+  fresh installs printed no next-run time at all, and mis-reported
+  UNIT/ACTIVATES when PASSED was a placeholder. The parser now
+  anchors on the trailing unit columns and treats placeholders
+  explicitly — both `-` and systemd 249's `n/a` — keeps the whole
+  multi-token LEFT field (`29min left`), and renders no next-run line
+  at all when the timer has no future elapse. `Uninstall` also
+  reports removal errors instead of silently claiming success, and is
+  a no-op again when nothing was installed.
+- **P2:** `systemd install` printed the volatile-binary-path warning
+  straight to stderr instead of routing it through `Status.Warnings`
+  (invisible to `--json`); `Uninstall` built a `removed` slice it
+  never read; a service unit with no `ExecStart=` line at all is now
+  flagged by the legacy-ExecStart warning.
+
+### Security
+
+- **P1:** The login/auto-refresh HTTP debug dump (`<< Set-Cookie: …`
+  values, request URLs) was printed **unconditionally** on every form
+  POST — `fetchPost` didn't take `Options`, and every auto-refresh
+  path hard-codes `Verbose: false`. Any `archive update` or `login`
+  run against an expired session therefore leaked the ADFS
+  `SAMLRequest` URL and raw `d2lSessionVal` / `d2lSecureSessionVal`
+  cookie values to stderr (and into journald under the systemd
+  unit), violating the threat model in `SECURITY.md`. `fetchPost` now
+  dumps only behind `--verbose`; every verbose URL print (request URL,
+  Referer, Location, redirect targets, POST form actions, GET
+  failures) redacts `SAMLRequest` / `SAMLResponse` / `RelayState` to
+  a length marker, and cookie values are redacted entirely rather
+  than truncated.
+- **P1:** An expired session during an archive run was reported as
+  generic failures: the my-courses listing 401'd as a course-list
+  error, a topic request that redirected to the D2L login page
+  (HTTP 200 + `text/html`) archived the login HTML as the topic's
+  metadata blob via the non-JSON fallback, a redirected file-body
+  download that ended at the login page hashed that HTML as the
+  topic's body, and a 401 on a body download was counted as an
+  ordinary body error so the run "succeeded" with missing bodies.
+  All of these paths now map to `errSessionExpired` and stop the run
+  with `session expired; run 'schooltools login'`.
 
 ## [0.2.0] - 2026-09-05
 

cmd/archive.go

@@ -102,8 +102,8 @@ func init() {
 
 	archiveFindCmd.Flags().StringVar(&archiveFindExt, "ext", "", "Filter by URL extension (e.g. pdf, docx)")
 	archiveFindCmd.Flags().StringVar(&archiveFindType, "type", "", "Filter by topic type (File, Link)")
-	archiveFindCmd.Flags().BoolVar(&archiveFindWith, "with-bodies", false, "Only show topics that have a file body archived")
-	archiveFindCmd.Flags().BoolVar(&archiveFindMiss, "missing-bodies", false, "Only show File topics whose body has never been archived")
+	archiveFindCmd.Flags().BoolVar(&archiveFindWith, "with-bodies", false, "Only show topics whose file body exists on disk")
+	archiveFindCmd.Flags().BoolVar(&archiveFindMiss, "missing-bodies", false, "Only show File topics whose file body is missing on disk")
 
 	archiveShowCmd.Flags().StringVar(&archiveShowKind, "kind", "", "Force the ref kind: metadata|body (default: auto-detect)")
 
@@ -236,6 +236,10 @@ var archiveUpdateCmd = &cobra.Command{
 		"body is missing on disk, even when its metadata is unchanged. Pass\n" +
 		"--no-bodies to skip body downloads. Pass --dry-run to preview without\n" +
 		"writing.\n\n" +
+		"Topics D2L flags as broken, and topics whose metadata fetch fails, are\n" +
+		"skipped and counted instead of aborting the pass — one dead topic can no\n" +
+		"longer stop the rest of your courses from archiving. A 401/403 response\n" +
+		"still stops the run with a \"session expired\" error.\n\n" +
 		"Examples:\n" +
 		"  schooltools archive update\n" +
 		"  schooltools archive update 12345 67890\n" +
@@ -309,6 +313,8 @@ func runArchiveUpdate(args []string) error {
 			"topicsChecked":  result.TopicsChecked,
 			"topicsFetched":  result.TopicsFetched,
 			"topicsStale":    result.TopicsStale,
+			"topicsFailed":   result.TopicsFailed,
+			"topicsSkipped":  result.TopicsSkipped,
 			"bodiesNew":      result.BodiesFetched,
 			"bodiesReused":   result.BodiesReused,
 			"bodiesFailed":   result.BodiesFailed,
@@ -321,8 +327,15 @@ func runArchiveUpdate(args []string) error {
 	fmt.Printf("Archive root: %s\n", result.Root)
 	fmt.Printf("Scanned %d course%s in %s\n",
 		result.CoursesScanned, plural(result.CoursesScanned), elapsed.Round(time.Millisecond))
-	fmt.Printf("Topics: %d checked, %d fetched, %d unchanged\n",
-		result.TopicsChecked, result.TopicsFetched, result.TopicsStale)
+	extra := ""
+	if result.TopicsSkipped > 0 {
+		extra += fmt.Sprintf(", %d skipped (broken)", result.TopicsSkipped)
+	}
+	if result.TopicsFailed > 0 {
+		extra += fmt.Sprintf(", %d failed", result.TopicsFailed)
+	}
+	fmt.Printf("Topics: %d checked, %d fetched, %d unchanged%s\n",
+		result.TopicsChecked, result.TopicsFetched, result.TopicsStale, extra)
 	if !archiveUpdateNoBodies && result.BodiesFetched+result.BodiesReused+result.BodiesFailed > 0 {
 		fmt.Printf("File bodies: %d new, %d unchanged (same SHA), %d errors\n",
 			result.BodiesFetched, result.BodiesReused, result.BodiesFailed)
@@ -367,6 +380,7 @@ func runArchiveUpdateDryRun(args []string, abs string) error {
 			"topicsChecked":    result.TopicsChecked,
 			"topicsWouldFetch": result.TopicsWouldFetch,
 			"topicsUnchanged":  result.TopicsStale,
+			"topicsBroken":     result.TopicsBroken,
 			"bodiesWouldFetch": result.BodiesWouldFetch,
 			"elapsedSeconds":   elapsed.Seconds(),
 			"perCourse":        result.PerCourse,
@@ -378,13 +392,21 @@ func runArchiveUpdateDryRun(args []string, abs string) error {
 	printUpdateDryRunTable(result)
 	fmt.Printf("\nWould scan %d course%s in %s\n",
 		result.CoursesScanned, plural(result.CoursesScanned), elapsed.Round(time.Millisecond))
-	fmt.Printf("Topics: %d checked, %d would fetch, %d unchanged\n",
-		result.TopicsChecked, result.TopicsWouldFetch, result.TopicsStale)
+	broken := ""
+	if result.TopicsBroken > 0 {
+		broken = fmt.Sprintf(" (%d broken topic%s never fetched)", result.TopicsBroken, plural(result.TopicsBroken))
+	}
+	fmt.Printf("Topics: %d checked, %d would fetch, %d unchanged%s\n",
+		result.TopicsChecked, result.TopicsWouldFetch, result.TopicsStale, broken)
 	if result.BodiesWouldFetch > 0 {
 		fmt.Printf("Bodies: %d would fill (convergence)\n", result.BodiesWouldFetch)
 	}
 	if result.TopicsWouldFetch == 0 && result.BodiesWouldFetch == 0 {
-		fmt.Println("Nothing to do — your archive is up to date.")
+		if result.TopicsBroken > 0 {
+			fmt.Println("Nothing to fetch — every remaining topic is flagged broken on D2L (see above); they will never be archived.")
+		} else {
+			fmt.Println("Nothing to do — your archive is up to date.")
+		}
 	} else {
 		fmt.Println("Re-run without --dry-run to apply.")
 	}
@@ -408,6 +430,7 @@ func printUpdateDryRunTable(result archive.DiffResult) {
 			fmt.Sprintf("%d", c.TopicsNew),
 			fmt.Sprintf("%d", c.TopicsModified),
 			fmt.Sprintf("%d", c.TopicsUnchanged),
+			fmt.Sprintf("%d", c.TopicsBroken),
 			fetch, bodies,
 		})
 	}
@@ -416,11 +439,11 @@ func printUpdateDryRunTable(result archive.DiffResult) {
 		return
 	}
 	out := table.Render(table.Options{
-		Headers: []string{"ID", "Code", "Name", "Total", "New", "Modified", "Unchanged", "WouldFetch", "Bodies"},
+		Headers: []string{"ID", "Code", "Name", "Total", "New", "Modified", "Unchanged", "Broken", "WouldFetch", "Bodies"},
 		Widths: table.Widths([]table.ColumnSpec{
 			table.Fixed(8), table.Fixed(8), table.Flex(20),
 			table.Fixed(6), table.Fixed(5), table.Fixed(9),
-			table.Fixed(10), table.Fixed(10), table.Flex(8),
+			table.Fixed(10), table.Fixed(6), table.Fixed(10), table.Flex(8),
 		}, table.TerminalWidth()),
 		Rows: rows,
 	})
@@ -596,8 +619,8 @@ var archiveFindCmd = &cobra.Command{
 		"type contains all whitespace-separated query tokens (case-insensitive).\n" +
 		"All tokens must match (AND). Use --ext to filter by file extension,\n" +
 		"--type File|Link to filter by topic type, --with-bodies for topics\n" +
-		"with an archived file body, --missing-bodies for File topics whose\n" +
-		"body has never been archived.\n\n" +
+		"whose file body exists on disk, --missing-bodies for File topics\n" +
+		"whose file body is missing on disk.\n\n" +
 		"Examples:\n" +
 		"  schooltools archive find safety contract\n" +
 		"  schooltools archive find --type File --ext pdf\n" +
@@ -614,12 +637,12 @@ func runArchiveFind(args []string) error {
 		return err
 	}
 	q := archive.SearchQuery{
-		Terms:         strings.Join(args, " "),
-		Ext:           archiveFindExt,
-		CourseIDs:     archiveCourses,
-		Type:          archiveFindType,
-		WithBodies:    archiveFindWith,
-		MissingBodies: archiveFindMiss,
+		Terms:            strings.Join(args, " "),
+		Ext:              archiveFindExt,
+		CourseIDs:        archiveCourses,
+		Type:             archiveFindType,
+		WithBodiesOnDisk: archiveFindWith,
+		MissingBodies:    archiveFindMiss,
 	}
 	hits := store.Search(q)
 

cmd/download.go

@@ -16,6 +16,7 @@ import (
 	"github.com/aarav/schooltools/internal/httpclient"
 	"github.com/aarav/schooltools/internal/session"
 	"github.com/aarav/schooltools/internal/ua"
+	"github.com/aarav/schooltools/internal/urlutil"
 )
 
 var (
@@ -316,21 +317,7 @@ func resolveFileURL(rawURL string) (string, error) {
 	if rawURL == "" {
 		return "", fmt.Errorf("empty url")
 	}
-	u, err := url.Parse(rawURL)
-	if err != nil {
-		return "", err
-	}
-	if u.IsAbs() {
-		return rawURL, nil
-	}
-	if u.Scheme == "" && u.Host == "" && (strings.HasPrefix(rawURL, "/") || strings.HasPrefix(rawURL, "./") || strings.HasPrefix(rawURL, "../")) {
-		base, perr := url.Parse(ua.D2LBase)
-		if perr != nil {
-			return "", perr
-		}
-		return base.ResolveReference(u).String(), nil
-	}
-	return rawURL, nil
+	return urlutil.ResolveD2L(ua.D2LBase, rawURL)
 }
 
 func pathExists(p string) (bool, error) {

cmd/download_test.go

@@ -0,0 +1,61 @@
+package cmd
+
+import (
+	"testing"
+)
+
+func TestResolveFileURL(t *testing.T) {
+	cases := []struct {
+		name    string
+		in      string
+		want    string
+		wantErr bool
+	}{
+		{
+			name: "relative enforced path with space and percent",
+			in:   "/content/enforced/1522695-x/Acetic Acid, 5% (Vinegar).pdf",
+			want: "https://d2l.cbe.ab.ca/content/enforced/1522695-x/Acetic%20Acid,%205%25%20(Vinegar).pdf",
+		},
+		{
+			name: "hash in filename becomes an escape, not a fragment",
+			in:   "/content/enforced/1527886-x/Quiz #3 Chemistry.pdf",
+			want: "https://d2l.cbe.ab.ca/content/enforced/1527886-x/Quiz%20%233%20Chemistry.pdf",
+		},
+		{
+			name: "existing escape preserved",
+			in:   "/content/enforced/1522695-x/KEY%20Practice.pdf",
+			want: "https://d2l.cbe.ab.ca/content/enforced/1522695-x/KEY%20Practice.pdf",
+		},
+		{
+			name: "relative reference with a colon in the first segment",
+			in:   "Week 1: Intro/notes.pdf",
+			want: "https://d2l.cbe.ab.ca/Week%201:%20Intro/notes.pdf",
+		},
+		{
+			name: "absolute URL is cleaned too",
+			in:   "https://files.example.com/a b#c.pdf",
+			want: "https://files.example.com/a%20b%23c.pdf",
+		},
+		{
+			name:    "empty URL",
+			in:      "",
+			wantErr: true,
+		},
+	}
+	for _, tc := range cases {
+		got, err := resolveFileURL(tc.in)
+		if tc.wantErr {
+			if err == nil {
+				t.Errorf("%s: resolveFileURL(%q) = %q, want an error", tc.name, tc.in, got)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("%s: resolveFileURL(%q): %v", tc.name, tc.in, err)
+			continue
+		}
+		if got != tc.want {
+			t.Errorf("%s: resolveFileURL(%q) = %q, want %q", tc.name, tc.in, got, tc.want)
+		}
+	}
+}

cmd/systemd.go

@@ -136,7 +136,11 @@ func printSystemdStatus(action string, st systemd.Status) error {
 			_, _ = fmt.Fprintf(tw, "Next run:\t%s (in %s)\n", st.Next, st.Left)
 		}
 		if st.Last != "" {
-			_, _ = fmt.Fprintf(tw, "Last run:\t%s (%s)\n", st.Last, st.Passed)
+			if st.Passed != "" {
+				_, _ = fmt.Fprintf(tw, "Last run:\t%s (%s)\n", st.Last, st.Passed)
+			} else {
+				_, _ = fmt.Fprintf(tw, "Last run:\t%s\n", st.Last)
+			}
 		}
 		writeWarnings()
 		_, _ = fmt.Fprintln(tw, "")
@@ -157,7 +161,11 @@ func printSystemdStatus(action string, st systemd.Status) error {
 			_, _ = fmt.Fprintf(tw, "Next run:\t%s (in %s)\n", st.Next, st.Left)
 		}
 		if st.Last != "" {
-			_, _ = fmt.Fprintf(tw, "Last run:\t%s (%s)\n", st.Last, st.Passed)
+			if st.Passed != "" {
+				_, _ = fmt.Fprintf(tw, "Last run:\t%s (%s)\n", st.Last, st.Passed)
+			} else {
+				_, _ = fmt.Fprintf(tw, "Last run:\t%s\n", st.Last)
+			}
 		}
 		if len(st.TimerOutput) > 0 {
 			_, _ = fmt.Fprintln(tw, "")