Skip to content

Commit 994a5d8

Browse files
authored
feat(core): implement path validation and traversal checks in HttpCommand (googleapis#27476)
Validate and encode Discovery path parameters in `HttpCommand` to protect REST clients from path traversal and query injection, following the cross-language REST URI special characters design. Summary of Changes - Rejects values that are exactly `.` or `..` in simple path variables (`{var}`). - Rejects segments that are exactly `.` or `..` in reserved path variables (`{+var}`). - Percent-encodes reserved path variables: every character except `[-_.~0-9a-zA-Z]` and the `/` segment separator (step 4 of the design). For example `?` becomes `%3F` and `:` becomes `%3A`. - Raises `Google::Apis::ClientError` when validation fails. How encoding works with Addressable `Addressable::Template` leaves reserved characters literal in `{+var}`, so `HttpCommand` encodes each reserved value before expansion and places it in the template's literal text, which Addressable passes through unchanged. Simple variables keep Addressable's encoding, which already percent-encodes every reserved character (`?` becomes `%3F`, `/` becomes `%2F`). Unicode normalization still follows `normalize_unicode`. Deviations from the design - **Simple variables:** The design recommends rejecting `/` in single-segment variables (step 1). This PR accepts it instead, because Addressable encodes `/` as `%2F`, which keeps the value in one path segment. Some APIs depend on this (for example Cloud Storage object names in `b/{bucket}/o/{object}`, where the object `folder/file.txt` expands to `b/my-bucket/o/folder%2Ffile.txt`).
1 parent 24eb281 commit 994a5d8

2 files changed

Lines changed: 264 additions & 1 deletion

File tree

‎google-apis-core/lib/google/apis/core/http_command.rb‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,18 @@ def authorization_refreshable?
160160
#
161161
# @private
162162
# @return [void]
163+
# @raise [Google::Apis::ClientError] If path parameter validation fails.
163164
def prepare!
164165
normalize_unicode = true
165166
if options
166167
header.update(options.header) if options.header
167168
query.update(options.query) if options.query
168169
normalize_unicode = options.normalize_unicode
169170
end
170-
self.url = url.expand(params, nil, normalize_unicode) if url.is_a?(Addressable::Template)
171+
if url.is_a?(Addressable::Template)
172+
validate_path_parameters!
173+
self.url = encode_reserved_parameters(url, normalize_unicode).expand(params, nil, normalize_unicode)
174+
end
171175
url.query_values = normalize_query_values(query).merge(url.query_values || {})
172176

173177
if allow_form_encoding?
@@ -355,6 +359,10 @@ def set_api_version_header(api_version)
355359
'Google::Apis::SecretmanagerV1beta1::SecretPayload'
356360
].freeze
357361

362+
# Matches the two URI template expressions Discovery documents use: simple ({var})
363+
# and reserved ({+var}). Group 1 is '+' for reserved expansion, group 2 the name.
364+
TEMPLATE_VAR_PATTERN = /\{(\+?)(\w+)\}/.freeze
365+
358366
module RedactingPPMethods
359367
def pp_object(obj)
360368
return super unless UNSAFE_CLASS_NAMES.include? obj.class.name
@@ -479,6 +487,96 @@ def normalize_query_value(v)
479487
v.to_s
480488
end
481489
end
490+
491+
# Validates user-supplied path parameter values against the URL template specification
492+
# to prevent directory traversal and parameter injection exploits.
493+
#
494+
# Validation rejects only dot segments. Other characters are percent-encoded instead.
495+
#
496+
# Validation Mechanism:
497+
# 1. For simple variables ({var}, standard single-wildcard behavior):
498+
# - Rejects if the value is exactly '.' or '..'.
499+
# - Reserved characters such as '/', '?' and '#' are percent-encoded by Addressable.
500+
# 2. For reserved variables ({+var}, double-wildcard behavior):
501+
# - Splits the value by slash ('/') and rejects any segment that is exactly '.' or '..'.
502+
# - Every character except '/' and unreserved characters is percent-encoded by
503+
# #encode_reserved_parameters.
504+
#
505+
# @raise [Google::Apis::ClientError] If any validation check fails.
506+
def validate_path_parameters!
507+
template_pattern = url.pattern
508+
509+
variables = template_pattern.scan(TEMPLATE_VAR_PATTERN).map do |operator, var_name|
510+
{ name: var_name, reserved: operator == '+' }
511+
end
512+
513+
variables.each do |v|
514+
var_name = v[:name]
515+
var_key = params.key?(var_name) ? var_name : var_name.to_sym
516+
next unless params.key?(var_key)
517+
518+
value = params[var_key].to_s
519+
520+
if v[:reserved]
521+
value.split('/').each do |seg|
522+
if ['.', '..'].include?(seg)
523+
raise Google::Apis::ClientError,
524+
"Value for #{var_name} must not contain segments that are exactly '#{seg}'."
525+
end
526+
end
527+
elsif ['.', '..'].include?(value)
528+
raise Google::Apis::ClientError,
529+
"Invalid value for #{var_name} '#{value}'."
530+
end
531+
end
532+
end
533+
534+
# Substitutes percent-encoded values for reserved ({+var}) variables in the template.
535+
#
536+
# Addressable leaves reserved characters such as '+', ':', '?' and '#' literal in a
537+
# reserved expansion, so they would reach the server unencoded. Only '/' and unreserved
538+
# characters should stay literal, so these values are encoded here and written into the
539+
# template's literal text, which Addressable passes through untouched when it expands
540+
# the rest.
541+
#
542+
# @param [Addressable::Template] template
543+
# The URL template.
544+
# @param [Boolean] normalize_unicode
545+
# Whether to apply Unicode NFC normalization to the values.
546+
# @return [Addressable::Template]
547+
# A template with reserved variables replaced by their encoded values.
548+
def encode_reserved_parameters(template, normalize_unicode)
549+
pattern = template.pattern.dup
550+
template.pattern.scan(TEMPLATE_VAR_PATTERN).each do |operator, var_name|
551+
next unless operator == '+'
552+
553+
var_key = params.key?(var_name) ? var_name : var_name.to_sym
554+
next unless params.key?(var_key)
555+
556+
# The encoded value contains no braces, so the new pattern parses cleanly and the
557+
# value cannot introduce a template expression of its own.
558+
pattern = pattern.gsub("{+#{var_name}}", encode_reserved_value(params[var_key], normalize_unicode))
559+
end
560+
Addressable::Template.new(pattern)
561+
end
562+
563+
# Percent-encodes every character except '/' and unreserved characters.
564+
#
565+
# @param [Object] value
566+
# The parameter value.
567+
# @param [Boolean] normalize_unicode
568+
# Whether to apply Unicode NFC normalization to the value.
569+
# @return [String]
570+
# The encoded value.
571+
def encode_reserved_value(value, normalize_unicode)
572+
value = value.to_s
573+
# Addressable only normalizes values left in the mapping, so normalize here to match.
574+
value = value.unicode_normalize(:nfc) if normalize_unicode
575+
# Split on '/' and keep empty segments (-1), so slashes stay literal.
576+
# Each segment is then fully encoded, including '%' itself (as '%25').
577+
unreserved = Addressable::URI::CharacterClasses::UNRESERVED
578+
value.split('/', -1).map { |segment| Addressable::URI.encode_component(segment, unreserved) }.join('/')
579+
end
482580
end
483581
end
484582
end

‎google-apis-core/spec/google/apis/core/http_command_spec.rb‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,4 +627,169 @@ class SecretPayload
627627
end
628628
end
629629

630+
describe 'path parameter validation' do
631+
# Runs the command against a stub and returns the path that went out on the wire.
632+
# WebMock treats '+' and '%2B' as equal, so the assertions check the expanded URL instead.
633+
def wire_path(command)
634+
stub_request(:get, /www\.googleapis\.com/).to_return(status: [200, ''])
635+
command.options.retries = 0
636+
command.execute(client)
637+
command.url.path
638+
end
639+
640+
context 'with simple path variables' do
641+
# Source template specification:
642+
# Pattern: v1/projects/{project}/locations/{location}/webhooks/{webhook}
643+
# Variable constraints (RFC 6570 simple expansion):
644+
# - Must not be exactly a traversal segment ('.', '..')
645+
# - Reserved characters such as '/', '?' and '#' are percent-encoded by Addressable
646+
let(:template) do
647+
Addressable::Template.new(
648+
'https://www.googleapis.com/v1/projects/{project}/locations/{location}/webhooks/{webhook}'
649+
)
650+
end
651+
652+
def webhook_command(webhook)
653+
command = Google::Apis::Core::HttpCommand.new(:get, template)
654+
command.params[:project] = 'sys-prod-123'
655+
command.params[:location] = 'us-central1'
656+
command.params[:webhook] = webhook
657+
command
658+
end
659+
660+
it 'should allow safe parameter values' do
661+
expect(wire_path(webhook_command('billing-webhook')))
662+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/billing-webhook')
663+
end
664+
665+
it 'should percent-encode slashes in simple parameters on the wire' do
666+
expect(wire_path(webhook_command('parent/child-webhook')))
667+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/parent%2Fchild-webhook')
668+
end
669+
670+
it 'should percent-encode slashes in Cloud Storage object names on the wire' do
671+
storage_template = Addressable::Template.new('https://www.googleapis.com/storage/v1/b/{bucket}/o/{object}')
672+
command = Google::Apis::Core::HttpCommand.new(:get, storage_template)
673+
command.params[:bucket] = 'bk'
674+
command.params[:object] = 'folder/file.txt'
675+
expect(wire_path(command)).to eq('/storage/v1/b/bk/o/folder%2Ffile.txt')
676+
end
677+
678+
it 'should reject dot or double-dot in simple parameters' do
679+
command = webhook_command('..')
680+
command.options.retries = 0
681+
expect { command.execute(client) }.to raise_error(Google::Apis::ClientError, "Invalid value for webhook '..'.")
682+
end
683+
684+
it 'should percent-encode percent signs in simple parameters on the wire' do
685+
expect(wire_path(webhook_command('%2e%2e')))
686+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/%252e%252e')
687+
expect(wire_path(webhook_command('%2e')))
688+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/%252e')
689+
end
690+
691+
it 'should percent-encode query and fragment characters in simple parameters on the wire' do
692+
expect(wire_path(webhook_command('billing-webhook?key=val')))
693+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/billing-webhook%3Fkey%3Dval')
694+
expect(wire_path(webhook_command('billing-webhook#fragment')))
695+
.to eq('/v1/projects/sys-prod-123/locations/us-central1/webhooks/billing-webhook%23fragment')
696+
end
697+
end
698+
699+
context 'with reserved path variables' do
700+
# Source template specification:
701+
# Pattern: v1/{+parent}/indexes
702+
# Variable constraints (RFC 6570 reserved expansion):
703+
# - Allows slashes (multiple path segments), including empty segments
704+
# - Must not contain traversal segments ('.', '..')
705+
# - Every character except '/' and unreserved characters is percent-encoded
706+
let(:template) { Addressable::Template.new('https://www.googleapis.com/v1/{+parent}/indexes') }
707+
708+
def parent_command(parent)
709+
command = Google::Apis::Core::HttpCommand.new(:get, template)
710+
command.params[:parent] = parent
711+
command
712+
end
713+
714+
def expect_rejected(parent, message)
715+
command = parent_command(parent)
716+
command.options.retries = 0
717+
expect { command.execute(client) }.to raise_error(Google::Apis::ClientError, message)
718+
end
719+
720+
it 'should allow safe double wildcard paths' do
721+
expect(wire_path(parent_command('projects/sys-prod-123/databases/default/documents/doc-1')))
722+
.to eq('/v1/projects/sys-prod-123/databases/default/documents/doc-1/indexes')
723+
end
724+
725+
it 'should percent-encode query characters in reserved parameters' do
726+
expect(wire_path(parent_command('projects/sys-prod-123?key=val')))
727+
.to eq('/v1/projects/sys-prod-123%3Fkey%3Dval/indexes')
728+
end
729+
730+
it 'should percent-encode fragment characters in reserved parameters' do
731+
expect(wire_path(parent_command('projects/sys-prod-123#fragment')))
732+
.to eq('/v1/projects/sys-prod-123%23fragment/indexes')
733+
end
734+
735+
it 'should percent-encode colons in reserved parameters' do
736+
expect(wire_path(parent_command('projects/p:undelete'))).to eq('/v1/projects/p%3Aundelete/indexes')
737+
end
738+
739+
it 'should percent-encode other reserved characters in reserved parameters' do
740+
expect(wire_path(parent_command('projects/p/topics/a+b@c,d=e')))
741+
.to eq('/v1/projects/p/topics/a%2Bb%40c%2Cd%3De/indexes')
742+
end
743+
744+
it 'should percent-encode braces in reserved parameters' do
745+
expect(wire_path(parent_command('projects/{x}'))).to eq('/v1/projects/%7Bx%7D/indexes')
746+
end
747+
748+
it 'should not normalize unicode in reserved parameters by default' do
749+
expect(wire_path(parent_command("projects/Cafe\u0301"))).to eq('/v1/projects/Cafe%CC%81/indexes')
750+
end
751+
752+
it 'should normalize unicode in reserved parameters when requested' do
753+
command = parent_command("projects/Cafe\u0301")
754+
command.options.normalize_unicode = true
755+
expect(wire_path(command)).to eq('/v1/projects/Caf%C3%A9/indexes')
756+
end
757+
758+
it 'should reject relative traversals inside the wildcard' do
759+
expect_rejected('projects/sys-prod-123/databases/default/documents/doc-1/../../default',
760+
"Value for parent must not contain segments that are exactly '..'.")
761+
end
762+
763+
it 'should reject traversals escaping the left parameter boundary' do
764+
expect_rejected('projects/sys-prod-123/databases/default/documents/doc-1/../../../../../../../escape-db',
765+
"Value for parent must not contain segments that are exactly '..'.")
766+
end
767+
768+
it 'should reject parameter values starting with parent directory traversals' do
769+
expect_rejected('../escape-db', "Value for parent must not contain segments that are exactly '..'.")
770+
end
771+
772+
it 'should reject invalid segments like single dot' do
773+
expect_rejected('projects/sys-prod-123/./databases/default',
774+
"Value for parent must not contain segments that are exactly '.'.")
775+
end
776+
777+
it 'should percent-encode percent signs in reserved parameters on the wire' do
778+
expect(wire_path(parent_command('projects/p/databases/d/documents/doc/%2e%2e/doc2')))
779+
.to eq('/v1/projects/p/databases/d/documents/doc/%252e%252e/doc2/indexes')
780+
expect(wire_path(parent_command('projects/p/databases/d/documents/doc/%2e/a')))
781+
.to eq('/v1/projects/p/databases/d/documents/doc/%252e/a/indexes')
782+
expect(wire_path(parent_command('projects/p/databases/d/documents/doc/..%2f..%2fescape-db')))
783+
.to eq('/v1/projects/p/databases/d/documents/doc/..%252f..%252fescape-db/indexes')
784+
expect(wire_path(parent_command('projects/p/databases/d/documents/doc/%2e%2e%2f%2e%2e%2fescape-db')))
785+
.to eq('/v1/projects/p/databases/d/documents/doc/%252e%252e%252f%252e%252e%252fescape-db/indexes')
786+
end
787+
788+
it 'should allow empty segments in reserved parameters' do
789+
expect(wire_path(parent_command('projects/sys-prod-123//databases/default')))
790+
.to eq('/v1/projects/sys-prod-123//databases/default/indexes')
791+
expect(wire_path(parent_command('projects/sys-prod-123/'))).to eq('/v1/projects/sys-prod-123//indexes')
792+
end
793+
end
794+
end
630795
end

0 commit comments

Comments
 (0)