Skip to content

fix: user search filters take a slug or 'me', not a numeric ID#9

Open
WebSearching404 wants to merge 1 commit into
ttpears:mainfrom
WebSearching404:fix/user-filter-slug-contract
Open

fix: user search filters take a slug or 'me', not a numeric ID#9
WebSearching404 wants to merge 1 commit into
ttpears:mainfrom
WebSearching404:fix/user-filter-slug-contract

Conversation

@WebSearching404

Copy link
Copy Markdown

The bug

validateUserIdFilters (introduced in 0b9dea4 / v3.5.0) enforces the inverse of BookStack's actual contract. BookStack resolves {created_by:X} / {updated_by:X} / {owned_by:X} by user slug, with 'me' as a shortcut for the token's own user — and when X matches no user's slug it silently skips the filter, returning all results as if they were filtered:

// app/Search/SearchRunner.php (v26.05.1, identical back through v25.x)
protected function filterCreatedBy(EloquentBuilder $query, string $input, bool $negated)
{
    $userSlug = $input === 'me' ? user()->slug : trim($input);
    $user = User::query()->where('slug', '=', $userSlug)->first(['id']);
    if ($user) {   // <- no match: filter silently dropped
        $this->applyNegatableWhere($query, $negated, 'created_by', '=', $user->id);
    }
}

https://github.com/BookStackApp/BookStack/blob/v26.05.1/app/Search/SearchRunner.php

So the current guard:

  • rejects every valid form of these filters — {created_by:me} and {created_by:some-slug} both throw, and
  • recommends the one form that fails open — a numeric ID only "works" if some user's slug happens to be that number; otherwise the query returns ALL results presented as filtered, which is precisely the footgun the guard was written to prevent.

Reproduced live on a seeded instance: {created_by:1} (admin's actual user ID) → returns everything, unfiltered; {created_by:me} and {created_by:admin} → filter correctly.

The fix

  • validateUserIdFilters now rejects pure-numeric values (the silent fail-open input), including the previously-unguarded negated {!field:N} form, and lets slugs / 'me' through. The error message explains the slug contract and points at find_users (which already returns the slug field).
  • The search and find_users tool descriptions and the README find_users row now state the slug contract instead of the numeric-ID one.

npm run build (tsc) passes; no schema or tool-surface changes.

Found while auditing a downstream fork, where the same fix has been running against a live v26.05.1 instance with an automated acceptance suite (including an empirical {created_by:me} filter check).

🤖 Generated with Claude Code

validateUserIdFilters (0b9dea4) enforced the inverse of BookStack's
actual contract: SearchRunner::filterCreatedBy/filterUpdatedBy/
filterOwnedBy resolve the filter value by user SLUG ('me' = the token's
own user) and silently skip the filter when nothing matches. So the
guard rejected every valid form of these filters (slugs, 'me') while
steering callers to numeric IDs — exactly the values BookStack ignores,
returning ALL results presented as filtered.

- validateUserIdFilters now rejects pure-numeric values (the silent
  fail-open input), including the previously-unguarded negated
  {!field:N} form, and accepts slugs/'me'
- search + find_users tool descriptions and the README row now state
  the slug contract

Verified against BookStack v26.05.1 source and live on a seeded
instance: {created_by:me} and {created_by:<slug>} filter correctly;
numeric IDs silently return unfiltered results.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant