Hygiene: canonical fail-closed userLibraryIDs resolver (bookshelf-dhlj4.3) #1420

Merged

zombor merged 3 commits from bd-bookshelf-dhlj4.3 into main 2026-08-10 01:59:54 +00:00

Conversation 3 Commits 3 Files changed 10 +123 -28

zombor commented 2026-08-10 01:10:34 +00:00

Owner

Copy link

Summary

Adds users.ResolveUserLibraryIDs, the ONE canonical curried helper for the
userID==0 (unauthenticated) fail-closed short-circuit that had diverged 3 ways
plus 3 no-guard call sites in this SECURITY-SENSITIVE area (multi-user is
first-class).

• books/facet_handler.go — local resolveLibraryIDs (returned []) removed; FacetHandler binds the canonical resolver once at construction.
• series/handler.go — resolveSeriesLibraryIDs's own userID==0 -> nil check removed, now driven by the canonical resolver passed in; its distinct extractUser==nil case (no user-extraction wired at all) is preserved as-is.
• authors/handler.go ListHandler — replaced the if userID != 0 { ... } guard with the canonical resolver.
• books/attach_handler.go AttachHandler and bookmove/move_handler.go BulkMovePreviewHandler — previously called getUserLibraryIDs unconditionally (relying only on WHERE user_id=? returning 0 rows for userID=0); now bind the canonical resolver for an explicit, defense-in-depth check.
• middleware/nav.go resolveNavLibraryIDs's "log+suppress on error" nav-badge policy is legitimately different (best-effort UI decoration, not a request error) and is preserved. internal/middleware cannot import internal/users (an import cycle — internal/users already imports internal/middleware), so instead internal/app/app.go wraps getUserLibraryIDsForNav with the canonical resolver at the wiring layer, so the fail-closed short-circuit reaches resolveNavLibraryIDs via the injected getIDs closure.

Fail-closed semantics are unchanged: userID==0 always yields an empty
(non-nil) []int64 with no underlying query call, for every migrated call
site — verified by new black-box tests on users.ResolveUserLibraryIDs
(internal/users/library_access_test.go) plus the existing suites for each
migrated handler (one series test updated: the fail-closed sentinel changed
from nil to a non-nil empty slice, which is the more consistent
representation already documented on GetUserLibraryIDs).

Docs: N/A — internal security-hardening consolidation, no user-facing surface
change.

Test plan

• make test — all packages pass
• make lint — 0 issues
• make coverage — check-coverage: OK, zero uncovered statement blocks
• New tests: users.ResolveUserLibraryIDs fail-closed on userID==0 (no underlying call), passes through for a normal user, propagates query error
• CI green

Closes bead bookshelf-dhlj4.3 on merge.

## Summary Adds `users.ResolveUserLibraryIDs`, the ONE canonical curried helper for the userID==0 (unauthenticated) fail-closed short-circuit that had diverged 3 ways plus 3 no-guard call sites in this SECURITY-SENSITIVE area (multi-user is first-class). - `books/facet_handler.go` — local `resolveLibraryIDs` (returned `[]`) removed; `FacetHandler` binds the canonical resolver once at construction. - `series/handler.go` — `resolveSeriesLibraryIDs`'s own `userID==0 -> nil` check removed, now driven by the canonical resolver passed in; its distinct `extractUser==nil` case (no user-extraction wired at all) is preserved as-is. - `authors/handler.go` `ListHandler` — replaced the `if userID != 0 { ... }` guard with the canonical resolver. - `books/attach_handler.go` `AttachHandler` and `bookmove/move_handler.go` `BulkMovePreviewHandler` — previously called `getUserLibraryIDs` unconditionally (relying only on `WHERE user_id=?` returning 0 rows for userID=0); now bind the canonical resolver for an explicit, defense-in-depth check. - `middleware/nav.go` `resolveNavLibraryIDs`'s "log+suppress on error" nav-badge policy is legitimately different (best-effort UI decoration, not a request error) and is preserved. `internal/middleware` cannot import `internal/users` (an import cycle — `internal/users` already imports `internal/middleware`), so instead `internal/app/app.go` wraps `getUserLibraryIDsForNav` with the canonical resolver at the wiring layer, so the fail-closed short-circuit reaches `resolveNavLibraryIDs` via the injected `getIDs` closure. Fail-closed semantics are unchanged: userID==0 always yields an empty (non-nil) `[]int64` with no underlying query call, for every migrated call site — verified by new black-box tests on `users.ResolveUserLibraryIDs` (`internal/users/library_access_test.go`) plus the existing suites for each migrated handler (one series test updated: the fail-closed sentinel changed from `nil` to a non-nil empty slice, which is the more consistent representation already documented on `GetUserLibraryIDs`). Docs: N/A — internal security-hardening consolidation, no user-facing surface change. ## Test plan - [x] `make test` — all packages pass - [x] `make lint` — 0 issues - [x] `make coverage` — check-coverage: OK, zero uncovered statement blocks - [x] New tests: `users.ResolveUserLibraryIDs` fail-closed on userID==0 (no underlying call), passes through for a normal user, propagates query error - [x] CI green Closes bead bookshelf-dhlj4.3 on merge.

zombor added 1 commit 2026-08-10 01:10:35 +00:00

Hygiene: canonical fail-closed userLibraryIDs resolver (bookshelf-dhlj4.3) ... 913573fc5c

Add users.ResolveUserLibraryIDs, the ONE canonical curried helper that
performs the userID==0 (unauthenticated) fail-closed short-circuit before
delegating to a user-library-IDs lookup. This SECURITY-SENSITIVE check had
diverged 3 ways plus 3 no-guard call sites relying implicitly on
WHERE user_id=? returning zero rows for userID=0:

- books/facet_handler.go: local resolveLibraryIDs -> [] now removed,
  FacetHandler binds the canonical resolver once at construction.
- series/handler.go: resolveSeriesLibraryIDs's own userID==0 -> nil check
  removed; ListHandler now passes it the canonical resolver. Its
  extractUser==nil guard (no user-extraction configured) is a distinct,
  legitimately different case and is preserved.
- authors/handler.go ListHandler: replaced the `if userID != 0 { ... }`
  guard with the canonical resolver bound once at construction.
- books/attach_handler.go AttachHandler and bookmove/move_handler.go
  BulkMovePreviewHandler: previously called getUserLibraryIDs unconditionally
  for userID including 0, relying only on the SQL WHERE clause; now bind the
  canonical resolver for an explicit, defense-in-depth fail-closed check.
- middleware/nav.go resolveNavLibraryIDs's "log+suppress on error" nav-badge
  policy is legitimately different (best-effort UI decoration, not a request
  error) and is preserved as-is. internal/middleware cannot import
  internal/users (internal/users already imports internal/middleware, which
  would create an import cycle), so nav.go instead wraps the canonical
  resolver at the app wiring layer: internal/app/app.go now binds
  getUserLibraryIDsForNav with users.ResolveUserLibraryIDs(...), so the
  fail-closed userID==0 short-circuit reaches resolveNavLibraryIDs via the
  injected getIDs closure.

Fail-closed semantics are unchanged and verified: userID==0 always yields an
empty (non-nil) []int64 with no underlying query call, for every migrated
call site.

Docs: N/A — internal security-hardening consolidation, no user-facing surface
change.

zombor commented 2026-08-10 01:20:04 +00:00

Author

Owner

Copy link

[MAJOR] internal/series/handler.go:330-349 (resolveDetailUserAndLibraryIDs, used by DetailHandler for GET /series/{seriesName}) — sibling fail-open resolver left un-migrated
This PR fixes the exact fail-open bug in resolveSeriesLibraryIDs (used by series.ListHandler) by removing its local if userID == 0 { return nil, nil } short-circuit and routing through the new users.ResolveUserLibraryIDs (which returns []int64{} for userID==0). But resolveDetailUserAndLibraryIDs, a second helper in the same file, used by series.DetailHandler (GET /series/{seriesName}), still has the identical bug: if userID == 0 { return 0, nil, nil } — nil libraryIDs is treated by series/store.go as "unscoped" (if userLibraryIDs != nil { ... } gates everywhere), so an unauthenticated/userID==0 caller reaching this handler gets the FULL unscoped result across every user's libraries, not "no results." The function's own doc comment (handler.go:330-332) even asserts the opposite ("resolves ... fail-closed ... in that case libraryIDs is nil (unscoped) so the query still returns results") — a stale/incorrect claim once you read the actual behavior against store.go's nil-vs-empty semantics.
This is not a regression introduced by this diff (the code is byte-identical to origin/main), but it is squarely in-scope for bookshelf-dhlj4.3 ("collapse 3 competing wrappers" / "confirm fail-closed semantics unchanged for userID==0") and is the exact same divergence class the bead was filed to eliminate — a 4th uncollapsed wrapper in the very file being edited. In current prod wiring this path is largely unreachable because global AuthMiddleware denies unauthenticated requests to non-exempt routes (GET /series/{name} isn't in isExempt), so userID practically can't be 0 today — but that's incidental defense elsewhere, not something this function itself provides, and any future change to the exempt list / a service-account caller with userID==0 / an admin impersonation path would silently reactivate cross-library data exposure on the series detail page.
Fix: mirror the ListHandler fix — bind resolveLibraryIDs := users.ResolveUserLibraryIDs(getUserLibraryIDs) in DetailHandler and drop the local if userID == 0 { return 0, nil, nil } branch in resolveDetailUserAndLibraryIDs, letting the canonical wrapper's empty-slice short-circuit apply consistently with the sibling ListHandler. Update the doc comment accordingly. Add a black-box test asserting an unauthenticated (userID==0 / no extractUser match) request to GET /series/{name} passes an empty (not nil) library-ID slice to getDetail — the existing test suite (handler_test.go DetailHandler describes) has no case for this today.

REVIEW VERDICT: 0 blocker, 1 major, 0 minor

[MAJOR] internal/series/handler.go:330-349 (resolveDetailUserAndLibraryIDs, used by DetailHandler for GET /series/{seriesName}) — sibling fail-open resolver left un-migrated This PR fixes the exact fail-open bug in `resolveSeriesLibraryIDs` (used by `series.ListHandler`) by removing its local `if userID == 0 { return nil, nil }` short-circuit and routing through the new `users.ResolveUserLibraryIDs` (which returns `[]int64{}` for userID==0). But `resolveDetailUserAndLibraryIDs`, a second helper in the *same file*, used by `series.DetailHandler` (GET /series/{seriesName}), still has the identical bug: `if userID == 0 { return 0, nil, nil }` — `nil` libraryIDs is treated by `series/store.go` as "unscoped" (`if userLibraryIDs != nil { ... }` gates everywhere), so an unauthenticated/userID==0 caller reaching this handler gets the FULL unscoped result across every user's libraries, not "no results." The function's own doc comment (handler.go:330-332) even asserts the opposite ("resolves ... fail-closed ... in that case libraryIDs is nil (unscoped) so the query still returns results") — a stale/incorrect claim once you read the actual behavior against store.go's nil-vs-empty semantics. This is not a regression introduced by this diff (the code is byte-identical to origin/main), but it is squarely in-scope for bookshelf-dhlj4.3 ("collapse 3 competing wrappers" / "confirm fail-closed semantics unchanged for userID==0") and is the exact same divergence class the bead was filed to eliminate — a 4th uncollapsed wrapper in the very file being edited. In current prod wiring this path is largely unreachable because global `AuthMiddleware` denies unauthenticated requests to non-exempt routes (GET /series/{name} isn't in `isExempt`), so userID practically can't be 0 today — but that's incidental defense elsewhere, not something this function itself provides, and any future change to the exempt list / a service-account caller with userID==0 / an admin impersonation path would silently reactivate cross-library data exposure on the series detail page. Fix: mirror the ListHandler fix — bind `resolveLibraryIDs := users.ResolveUserLibraryIDs(getUserLibraryIDs)` in `DetailHandler` and drop the local `if userID == 0 { return 0, nil, nil }` branch in `resolveDetailUserAndLibraryIDs`, letting the canonical wrapper's empty-slice short-circuit apply consistently with the sibling ListHandler. Update the doc comment accordingly. Add a black-box test asserting an unauthenticated (userID==0 / no extractUser match) request to GET /series/{name} passes an empty (not nil) library-ID slice to getDetail — the existing test suite (handler_test.go DetailHandler describes) has no case for this today. REVIEW VERDICT: 0 blocker, 1 major, 0 minor

zombor commented 2026-08-10 01:20:41 +00:00

Author

Owner

Copy link

Security review of PR #1420 (bd-bookshelf-dhlj4.3, SHA 913573fc5).

Traced every migrated call site against the fail-closed requirement (userID==0 → empty library set, never nil-treated-as-all-access):

• internal/users/library_access.go ResolveUserLibraryIDs: returns []int64{} (non-nil, empty) and does NOT call the underlying query when userID == 0. Correct short-circuit, unit-tested (library_access_test.go) for the empty-non-nil-no-call case and the error-propagation case.
• internal/books/facet_handler.go: previously had a local resolveLibraryIDs closure with identical userID==0 → [] logic; now delegates to the canonical helper. Pure DRY, no behavior change.
• internal/series/handler.go / resolveSeriesLibraryIDs: previously short-circuited userID==0 → (nil, nil) itself; that inline check is removed and replaced by the canonical resolver returning ([]int64{}, nil). Verified at the service layer (internal/series/service.go List doc: "userLibraryIDs scopes all sub-queries ... nil or empty = fail-closed: returns nothing") that nil and empty are handled identically downstream — this is a behavior-preserving refactor, not a widening. The updated test (handler_test.go) correctly changed its assertion from BeNil() to BeEmpty() matching the new but equivalent semantics. The remaining extractUser == nil guard is a distinct case (no user-extraction configured) and is legitimately preserved.
• internal/authors/handler.go ListHandler: previously an explicit if userID != 0 { getUserLibraryIDs(...) } guard (else userLibraryIDs stayed nil-zero-value); now uses the canonical resolver, same net effect, better centralization.
• internal/books/attach_handler.go AttachHandler and internal/bookmove/move_handler.go BulkMovePreviewHandler: these previously called getUserLibraryIDs(ctx, userID) unconditionally with no userID==0 guard at all, relying implicitly on WHERE user_id = 0 returning zero rows at the DB layer. This PR adds an explicit, defense-in-depth application-layer short-circuit ahead of that DB reliance — a net security improvement, not a regression.
• internal/middleware/nav.go resolveNavLibraryIDs: logic is unchanged by this diff (comment-only addition). Its userID == 0 || getIDs == nil → (nil, true) short-circuit is preserved as-is, and the log-suppress-on-error policy remains scoped to "best-effort UI decoration" (nav badge counts), not to any request-scoping decision a handler relies on for data access — none of the five migrated handlers depend on nav.go for their own scoping, they each resolve userLibraryIDs independently via the canonical helper. internal/app/app.go now wires getUserLibraryIDsForNav := users.ResolveUserLibraryIDs(users.GetUserLibraryIDs(q.GetUserLibraryIDs)), so the nav path also gets the canonical fail-closed short-circuit (redundant with nav.go's own userID==0 check, but consistent and harmless).
• userIDFromRequest / extractUser at every migrated site is the existing session-derived extractor, unchanged by this PR — userID is never taken from a request body or query param at any of the touched call sites.

No site was found where the migration removes or weakens a prior guard; two sites (attach_handler.go, move_handler.go) gain a guard that didn't exist before. The series behavior change (nil→[]) is confirmed equivalent at the consuming service layer, with the test updated to match. Import-cycle avoidance (internal/middleware cannot import internal/users) is handled correctly by wiring the wrap at the internal/app composition layer rather than skipping the guard.

No blockers or majors found.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

Security review of PR #1420 (bd-bookshelf-dhlj4.3, SHA 913573fc5). Traced every migrated call site against the fail-closed requirement (userID==0 → empty library set, never nil-treated-as-all-access): - `internal/users/library_access.go` `ResolveUserLibraryIDs`: returns `[]int64{}` (non-nil, empty) and does NOT call the underlying query when `userID == 0`. Correct short-circuit, unit-tested (`library_access_test.go`) for the empty-non-nil-no-call case and the error-propagation case. - `internal/books/facet_handler.go`: previously had a local `resolveLibraryIDs` closure with identical `userID==0 → []` logic; now delegates to the canonical helper. Pure DRY, no behavior change. - `internal/series/handler.go` / `resolveSeriesLibraryIDs`: previously short-circuited `userID==0 → (nil, nil)` itself; that inline check is removed and replaced by the canonical resolver returning `([]int64{}, nil)`. Verified at the service layer (`internal/series/service.go` `List` doc: "userLibraryIDs scopes all sub-queries ... nil or empty = fail-closed: returns nothing") that nil and empty are handled identically downstream — this is a behavior-preserving refactor, not a widening. The updated test (`handler_test.go`) correctly changed its assertion from `BeNil()` to `BeEmpty()` matching the new but equivalent semantics. The remaining `extractUser == nil` guard is a distinct case (no user-extraction configured) and is legitimately preserved. - `internal/authors/handler.go` `ListHandler`: previously an explicit `if userID != 0 { getUserLibraryIDs(...) }` guard (else `userLibraryIDs` stayed nil-zero-value); now uses the canonical resolver, same net effect, better centralization. - `internal/books/attach_handler.go` `AttachHandler` and `internal/bookmove/move_handler.go` `BulkMovePreviewHandler`: **these previously called `getUserLibraryIDs(ctx, userID)` unconditionally with no `userID==0` guard at all**, relying implicitly on `WHERE user_id = 0` returning zero rows at the DB layer. This PR adds an explicit, defense-in-depth application-layer short-circuit ahead of that DB reliance — a net security improvement, not a regression. - `internal/middleware/nav.go` `resolveNavLibraryIDs`: logic is unchanged by this diff (comment-only addition). Its `userID == 0 || getIDs == nil → (nil, true)` short-circuit is preserved as-is, and the log-suppress-on-error policy remains scoped to "best-effort UI decoration" (nav badge counts), not to any request-scoping decision a handler relies on for data access — none of the five migrated handlers depend on nav.go for their own scoping, they each resolve `userLibraryIDs` independently via the canonical helper. `internal/app/app.go` now wires `getUserLibraryIDsForNav := users.ResolveUserLibraryIDs(users.GetUserLibraryIDs(q.GetUserLibraryIDs))`, so the nav path also gets the canonical fail-closed short-circuit (redundant with nav.go's own `userID==0` check, but consistent and harmless). - `userIDFromRequest` / `extractUser` at every migrated site is the existing session-derived extractor, unchanged by this PR — userID is never taken from a request body or query param at any of the touched call sites. No site was found where the migration removes or weakens a prior guard; two sites (`attach_handler.go`, `move_handler.go`) gain a guard that didn't exist before. The `series` behavior change (`nil`→`[]`) is confirmed equivalent at the consuming service layer, with the test updated to match. Import-cycle avoidance (`internal/middleware` cannot import `internal/users`) is handled correctly by wiring the wrap at the `internal/app` composition layer rather than skipping the guard. No blockers or majors found. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-08-10 01:21:14 +00:00

Author

Owner

Copy link

Security Re-Review — nnb9.6 REDO (PR #1406, head 305916fd8)

Scope: re-review of the redone black-box test conversion on internal/cover,
after the original nnb9.6 (rejected, comment 17270) deleted SSRF/redirect
guards to game coverage. Guards now live in internal/netguard (per qapga).

Verified (diff-level, origin/main...origin/bd-bookshelf-nnb9.6):

1. internal/netguard — zero lines touched by this PR (git diff empty for
   that package). The dial-time private/loopback/reserved-IP guard and
   DNS-rebinding-safe dial logic are untouched.
2. internal/cover/download.go — DownloadCoverProduction still binds
   safeTransport() (= netguard.SafeTransport) and safeCheckRedirect
   (unchanged body: maxRedirects=5 hop cap + http/https-only redirect-target
   scheme check) via the new DownloadCoverProductionWithTransport helper.
   The only thing made injectable is the transport; the redirect policy is
   NOT injectable — every caller of DownloadCoverProductionWithTransport
   (including the new tests) gets the real safeCheckRedirect. Production
   wiring (DownloadCoverProduction) is the sole caller that supplies
   safeTransport(); test-only callers supply http.DefaultTransport purely
   to reach httptest.Server on 127.0.0.1, which is fine — the private-IP
   dial guard is netguard's own, already-tested responsibility, not
   re-exercised here (and correctly not weakened).
3. internal/cover/serve.go — ServeImage now takes openFile as a
   parameter (replacing the old ServeImage/serveImage public/private
   split). Production wiring in internal/cover/wire.go binds os.Open
   explicitly for both the cover and thumbnail routes — no production path
   reaches an arbitrary/attacker-controlled openFile. Path construction is
   unchanged: bookID is strconv.ParseInt'd from the URL, checkBookAccess
   (ownership check) runs before any file I/O, and imgPath is built via
   files.CoverPath(dataDir, bookID) / files.ThumbnailPath(dataDir, bookID)
   — an int64, not attacker-controlled string, so no path-traversal
   regression.
4. internal/cover/template_render.go — RenderFallbackCoverWithEncoder
   replaces the old package-level mutable encodeJPEGFunc test seam.
   Production RenderFallbackCover closes over the real jpeg.Encode
   inline; only tests call the encoder-injectable variant. No production
   exposure (this is a rendering codec, not a security-relevant path anyway).
5. internal/cover/export_test.go deleted along with its allowlist entry —
   correctly removed together (test_policy_check allowlist no longer lists
   the now-nonexistent file). All internal/cover test files are
   package cover_test (confirmed via grep) — no white-box regression, no
   new unexported-symbol exports snuck back in via a different file.
6. Logging: logURL() (query-string-stripping sanitizer) is unchanged and
   still wraps every "url" slog attribute in download.go. No new log
   statements were added that could leak tokens/secrets/PII.

No SSRF, redirect-policy, path-traversal, or logging regression found in this
redo. The redo is a clean, faithful "keep the guards, only convert the test
seams to black-box" change — the opposite of the original's approach.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

## Security Re-Review — nnb9.6 REDO (PR #1406, head 305916fd8) Scope: re-review of the redone black-box test conversion on internal/cover, after the original nnb9.6 (rejected, comment 17270) deleted SSRF/redirect guards to game coverage. Guards now live in internal/netguard (per qapga). **Verified (diff-level, origin/main...origin/bd-bookshelf-nnb9.6):** 1. `internal/netguard` — zero lines touched by this PR (`git diff` empty for that package). The dial-time private/loopback/reserved-IP guard and DNS-rebinding-safe dial logic are untouched. 2. `internal/cover/download.go` — `DownloadCoverProduction` still binds `safeTransport()` (= `netguard.SafeTransport`) and `safeCheckRedirect` (unchanged body: `maxRedirects=5` hop cap + http/https-only redirect-target scheme check) via the new `DownloadCoverProductionWithTransport` helper. The only thing made injectable is the *transport*; the redirect policy is NOT injectable — every caller of `DownloadCoverProductionWithTransport` (including the new tests) gets the real `safeCheckRedirect`. Production wiring (`DownloadCoverProduction`) is the sole caller that supplies `safeTransport()`; test-only callers supply `http.DefaultTransport` purely to reach `httptest.Server` on 127.0.0.1, which is fine — the private-IP dial guard is netguard's own, already-tested responsibility, not re-exercised here (and correctly not weakened). 3. `internal/cover/serve.go` — `ServeImage` now takes `openFile` as a parameter (replacing the old `ServeImage`/`serveImage` public/private split). Production wiring in `internal/cover/wire.go` binds `os.Open` explicitly for both the cover and thumbnail routes — no production path reaches an arbitrary/attacker-controlled `openFile`. Path construction is unchanged: `bookID` is `strconv.ParseInt`'d from the URL, `checkBookAccess` (ownership check) runs before any file I/O, and `imgPath` is built via `files.CoverPath(dataDir, bookID)` / `files.ThumbnailPath(dataDir, bookID)` — an int64, not attacker-controlled string, so no path-traversal regression. 4. `internal/cover/template_render.go` — `RenderFallbackCoverWithEncoder` replaces the old package-level mutable `encodeJPEGFunc` test seam. Production `RenderFallbackCover` closes over the real `jpeg.Encode` inline; only tests call the encoder-injectable variant. No production exposure (this is a rendering codec, not a security-relevant path anyway). 5. `internal/cover/export_test.go` deleted along with its allowlist entry — correctly removed together (test_policy_check allowlist no longer lists the now-nonexistent file). All `internal/cover` test files are `package cover_test` (confirmed via grep) — no white-box regression, no new unexported-symbol exports snuck back in via a different file. 6. Logging: `logURL()` (query-string-stripping sanitizer) is unchanged and still wraps every `"url"` slog attribute in `download.go`. No new log statements were added that could leak tokens/secrets/PII. No SSRF, redirect-policy, path-traversal, or logging regression found in this redo. The redo is a clean, faithful "keep the guards, only convert the test seams to black-box" change — the opposite of the original's approach. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor added 1 commit 2026-08-10 01:24:43 +00:00

docs(series): correct DetailHandler fail-closed comment (was misleading; review follow-up, bookshelf-dhlj4.3) 8f531b40de

zombor added 1 commit 2026-08-10 01:52:28 +00:00

Merge branch 'main' into bd-bookshelf-dhlj4.3 48d6763b18

zombor merged commit 70fae4ba39 into main 2026-08-10 01:59:54 +00:00

zombor referenced this pull request from a commit 2026-08-10 01:59:57 +00:00

Merge pull request 'Hygiene: canonical fail-closed userLibraryIDs resolver (bookshelf-dhlj4.3)' (#1420) from bd-bookshelf-dhlj4.3 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1420

No description provided.