Skip to content

feat(core): implement path validation and traversal checks in HttpCommand - #27476

Open
quartzmo wants to merge 11 commits into
googleapis:mainfrom
quartzmo:discovery-core-validation
Open

quartzmo wants to merge 11 commits into
googleapis:mainfrom
quartzmo:discovery-core-validation

Conversation

@quartzmo

@quartzmo quartzmo commented Jul 27, 2026 •

Copy link
Copy Markdown
Member

Implement dynamic runtime template parsing and validation inside HttpCommand to protect Discovery REST clients from path traversal and parameter injection exploits.

Summary of Changes

  • Rejects query (?) and fragment (#) characters in all path parameter values.
  • Rejects slashes (/) and exact . or .. values in simple path variables (standard wildcards {var}).
  • Rejects empty segments (consecutive // or trailing slashes) and any path segments that are exactly . or .. in reserved path variables (double wildcards {+var} and {#var}).
  • Adds unit test assertions verifying that percent-encoded characters (like %2e%2e and ..%2f..) are safely encoded to %252e%252e and ..%252f.. on the wire by Addressable::Template.
  • Standardizes path validation error messages to align with gapic-common and .NET client library formats.

Addressable Expansion Behavior

In Ruby Discovery clients, URI template expansion is handled by Addressable::Template. Because RFC 6570 does not include % in the RESERVED set, Addressable::Template#expand encodes % to %25 on the wire (e.g. %2e%2e expands to %252e%252e).

This ensures that percent-encoded characters remain literal string data when decoded by downstream HTTP servers and reverse proxies (GFE, Envoy, Rack), preventing obfuscated directory traversal attempts.

…mand

Implement dynamic runtime template parsing and validation inside HttpCommand
to protect Discovery REST clients from path traversal and parameter injection exploits.

Specifically, this change:
- Rejects query (?) and fragment (#) characters in path parameter values.
- Rejects slashes (/) and dots (. or ..) in simple path variables (standard wildcards).
- Validates path traversals in reserved path variables (+ or #, double wildcards)
  using a segment-boundary traversal validation algorithm.

In addition to the validation logic, this commit includes:
- Ruby style modernization: cleaned up and auto-corrected string quote style to favor single quotes where appropriate.
- Formatted long stub requests to follow style guidelines.

Note: If the canonical specification is updated to favor the simpler alternative,
this implementation can be easily simplified to "Fail always if dots are found in
a double-wildcard value" by replacing the segment-traversal logic with a simple
check for any "." or ".." substring.
@quartzmo
quartzmo force-pushed the discovery-core-validation branch from 66dff5d to c84404d Compare August 4, 2026 22:43
@quartzmo
quartzmo marked this pull request as ready for review August 4, 2026 23:55
@quartzmo
quartzmo requested a review from a team as a code owner August 4, 2026 23:55

@torreypayne torreypayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome! The only thing I'm wondering is if there are any test cases we could extract from the original vulnerability ask to ensure we have coverage. Otherwise LGTM!

Comment thread google-apis-core/lib/google/apis/core/http_command.rb Outdated
Comment thread google-apis-core/lib/google/apis/core/http_command.rb Outdated
Comment thread google-apis-core/lib/google/apis/core/http_command.rb Outdated
Comment thread google-apis-core/lib/google/apis/core/http_command.rb
Comment thread google-apis-core/spec/google/apis/core/http_command_spec.rb Outdated
Comment thread google-apis-core/lib/google/apis/core/http_command.rb Outdated
Comment thread google-apis-core/lib/google/apis/core/http_command.rb Outdated
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.

4 participants