Skip to content

[26.0] Harden API parameter validation#22351

Merged
mvdbeek merged 11 commits intogalaxyproject:release_26.0from
mvdbeek:harden_api_parameter_validation
Apr 3, 2026
Merged

[26.0] Harden API parameter validation#22351
mvdbeek merged 11 commits intogalaxyproject:release_26.0from
mvdbeek:harden_api_parameter_validation

Conversation

@mvdbeek
Copy link
Copy Markdown
Member

@mvdbeek mvdbeek commented Apr 1, 2026

Summary

Systematic hardening of API input validation so malformed/malicious requests are rejected at the API frontier with 4xx responses instead of crashing deep in application code as 500s logged to Sentry.

Full diagnosis plan: https://gist.github.com/mvdbeek/9fcae5ac8e5a4e77abef119c01dcbb08

Commits

Commit Fixes Description
ac595d6 #22350 Validate since parameter before parsing as ISO date
93e690a #22349 Return 400 for invalid genome build ID instead of 500
38f0c1a #22348 Handle missing payload in ToolsController.create()
4afff17 #22347 Require authentication for WES list_runs endpoint
82574ac #22346 Validate UUID format before dynamic tool lookup
4f60a2a #22345 Harden gxfiles:// URI validation against embedded schemes
a22d4c0 #22344 Validate payload type in _kwd_or_payload
65ecb96 #22343 Remove unused client metrics endpoint
2298130 #22342 Handle UnicodeDecodeError in cookie parsing gracefully
646ce87 #22341 Reject invalid object store IDs at the API frontier

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

mvdbeek added 6 commits April 1, 2026 11:19
Two issues caused flaky failures in test_tool_discovery_landing:

1. fetchHelpForId used a Set to guard against duplicate fetches. The
   watchEffect in ToolsListTable called fetchHelpForId fire-and-forget,
   adding tool IDs to the Set immediately. When loadTools subsequently
   called fetchHelpForId for the same IDs, the guard returned a resolved
   Promise before data was actually cached, so await Promise.all()
   resolved prematurely and help toggle links never appeared.

   Fix: change fetchedHelpIds from Set to Map<string, Promise<void>> so
   concurrent callers share the same in-flight Promise.

2. After a search, ScrollList kept stale items in localItems and
   considered allLoaded=true (old item count >= new total), so loadTools
   was never re-invoked for the filtered results. Help data was never
   awaited for the search results.

   Fix: replace the fire-and-forget watchEffect with a computed key on
   ScrollList that forces a remount when props.tools changes, ensuring
   loadTools runs again and awaits help data for new results.
When a client passes a nonexistent genome build ID (e.g. "1"), return a
400 RequestParameterInvalidException instead of a 500 ReferenceDataError.
The latter is reserved for genuine server-side reference data issues.

Fixes galaxyproject#22349
Return 400 instead of 500 TypeError when POST /api/tools is called
without a request body.

Fixes galaxyproject#22348
Anonymous requests crash with AttributeError when accessing trans.user.id.
Return 401 instead.

Fixes galaxyproject#22347
Reject badly formed UUID strings with a 400 instead of letting them
propagate as a StatementError (500) from SQLAlchemy.

Fixes galaxyproject#22346
mvdbeek added 5 commits April 1, 2026 11:47
- Fix score_url_match to use prefix matching (startswith) instead of
  substring matching (in), preventing malicious URIs like
  gxfiles://stock_httphttp://evil.com from matching legitimate sources.
- Reject URIs containing embedded :// schemes in the remote files manager.
- Add unit test for the prefix matching behavior.

Fixes galaxyproject#22345
Return 400 when payload is not a dict (e.g. a raw string) instead of
crashing with AttributeError.

Fixes galaxyproject#22344
The POST /api/metrics endpoint was a never-completed Fluentd bridge stub
with no frontend callers. With fluent_log=false (default), it only called
log.debug() and returned {}. No feature development in 3+ years.

Fixes galaxyproject#22343
When cookie bytes contain invalid UTF-8 (e.g. from tampered requests or
mangling proxies), catch UnicodeDecodeError and treat as no cookies
present. Galaxy already handles missing cookies by creating a new
anonymous session.

Fixes galaxyproject#22342
- Add regex pattern constraint on ConcreteObjectStoreIdPathParam to only
  accept word characters and hyphens, blocking path traversal attempts.
- Fix DistributedObjectStore.get_concrete_store_by_object_store_id to
  return None instead of raising KeyError, matching the base class
  contract so _model_for returns 404.

Fixes galaxyproject#22341
@mvdbeek mvdbeek force-pushed the harden_api_parameter_validation branch from 90c0680 to 646ce87 Compare April 1, 2026 09:47
@mvdbeek
Copy link
Copy Markdown
Member Author

mvdbeek commented Apr 1, 2026

Test errors unrelated

@mvdbeek mvdbeek requested a review from a team April 1, 2026 13:02
@mvdbeek mvdbeek merged commit 001d6fd into galaxyproject:release_26.0 Apr 3, 2026
59 of 61 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in Galaxy Dev - weeklies Apr 3, 2026
@ahmedhamidawan ahmedhamidawan modified the milestones: 26.1, 26.0 Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants