Skip to content

Commit 5fc2a81

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(platform): refuse query and fragment in mirror and factory URLs
The mirror endpoint pattern and the factory URL check keep user:password@ out of the cozystack-values Secret, which is copied into every tenant namespace, but a token in the query or the fragment passes both: the host class of the endpoint pattern accepts ? and #, and the factory URL check looks for an @ before the first slash only. Parse the URL and refuse a non-empty user info, query or fragment, for the mirror endpoints and for kubernetesWorkerImage.imageFactoryURL, which is copied the same way. This also puts the urlParse result to use; it was assigned and never read. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 4e8dae2 commit 5fc2a81

4 files changed

Lines changed: 65 additions & 4 deletions

File tree

‎packages/core/platform/templates/apps.yaml‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,14 @@
5252
{{- fail (printf "kubernetesRegistries.mirrors[%q].endpoints must hold URL strings, got %v" $host .) -}}
5353
{{- end -}}
5454
{{- if not (regexMatch "(?i)^https?://[^/@[:space:]]+(/[^[:space:]]*)?$" .) -}}
55-
{{- fail (printf "kubernetesRegistries.mirrors[%q].endpoints entry %q must be an http(s) URL with no user name or password in it: the mirrors are copied into the cozystack-values Secret of every tenant." $host .) -}}
55+
{{- fail (printf "kubernetesRegistries.mirrors[%q].endpoints entry %q must be an http(s) URL with no user name or password, query or fragment in it: the mirrors are copied into the cozystack-values Secret of every tenant." $host .) -}}
5656
{{- end -}}
57+
{{- /* The pattern keeps user:password@ out, but a token can ride in the query or the
58+
fragment, and both pass it. */ -}}
5759
{{- $parsed := urlParse . -}}
60+
{{- if or $parsed.userinfo $parsed.query $parsed.fragment -}}
61+
{{- fail (printf "kubernetesRegistries.mirrors[%q].endpoints entry %q must be an http(s) URL with no user name or password, query or fragment in it: the mirrors are copied into the cozystack-values Secret of every tenant." $host .) -}}
62+
{{- end -}}
5863
{{- end -}}
5964
{{- end -}}
6065
{{- $kubernetesRegistriesConfig := $kubernetesRegistries.config | default dict -}}
@@ -407,6 +412,13 @@ stringData:
407412
{{- if and $workerFactoryURL (regexMatch "^https?://[^/]*@" $workerFactoryURL) }}
408413
{{- fail "kubernetesWorkerImage.imageFactoryURL must not carry a user name or password: it is copied into the cozystack-values Secret of every tenant and into the spec of every new worker pool." }}
409414
{{- end }}
415+
{{- /* A token can ride in the query or the fragment as well, and the patterns above let both through. */}}
416+
{{- if $workerFactoryURL }}
417+
{{- $workerFactoryParsed := urlParse $workerFactoryURL }}
418+
{{- if or $workerFactoryParsed.userinfo $workerFactoryParsed.query $workerFactoryParsed.fragment }}
419+
{{- fail "kubernetesWorkerImage.imageFactoryURL must not carry a user name, password, query or fragment: it is copied into the cozystack-values Secret of every tenant and into the spec of every new worker pool." }}
420+
{{- end }}
421+
{{- end }}
410422
{{- $workerInstaller := $workerImage.installerRepository | default "" | toString | trimSuffix "/" }}
411423
{{- /* A registry host with an optional port, then repository path components,
412424
which an image reference takes in lower case. A scheme is refused because

‎packages/core/platform/tests/apps_kubernetes_registries_wiring_test.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,39 @@ tests:
157157
- failedTemplate:
158158
errorPattern: 'must be an http\(s\) URL with no user name or password'
159159

160+
- it: refuses an endpoint that carries a token in the query
161+
set:
162+
kubernetesRegistries:
163+
mirrors:
164+
docker.io:
165+
endpoints:
166+
- https://mirror.example.org?token=SECRET
167+
asserts:
168+
- failedTemplate:
169+
errorPattern: 'must be an http\(s\) URL with no user name or password, query or fragment'
170+
171+
- it: refuses an endpoint that carries a token in the query of a path
172+
set:
173+
kubernetesRegistries:
174+
mirrors:
175+
docker.io:
176+
endpoints:
177+
- https://mirror.example.org/v2/?access_token=SECRET
178+
asserts:
179+
- failedTemplate:
180+
errorPattern: 'must be an http\(s\) URL with no user name or password, query or fragment'
181+
182+
- it: refuses an endpoint that carries a token in the fragment
183+
set:
184+
kubernetesRegistries:
185+
mirrors:
186+
docker.io:
187+
endpoints:
188+
- https://mirror.example.org/v2/#SECRET
189+
asserts:
190+
- failedTemplate:
191+
errorPattern: 'must be an http\(s\) URL with no user name or password, query or fragment'
192+
160193
- it: refuses an endpoint without a scheme
161194
set:
162195
kubernetesRegistries:

‎packages/core/platform/tests/apps_worker_image_defaults_wiring_test.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,22 @@ tests:
212212
- failedTemplate:
213213
errorPattern: 'imageFactoryURL must not carry a user name or password'
214214

215+
- it: refuses a factory URL that carries a token in the query
216+
set:
217+
kubernetesWorkerImage:
218+
imageFactoryURL: https://factory.example.com?token=SECRET
219+
asserts:
220+
- failedTemplate:
221+
errorPattern: 'imageFactoryURL must not carry a user name, password, query or fragment'
222+
223+
- it: refuses a factory URL that carries a token in the fragment
224+
set:
225+
kubernetesWorkerImage:
226+
imageFactoryURL: https://factory.example.com/#SECRET
227+
asserts:
228+
- failedTemplate:
229+
errorPattern: 'imageFactoryURL must not carry a user name, password, query or fragment'
230+
215231
- it: refuses whitespace in a factory URL
216232
set:
217233
kubernetesWorkerImage:

‎packages/core/platform/values.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -765,9 +765,9 @@ backupStrategyController: {}
765765
# diskSize below its storage. Such a pool sets `osImage.factory: {}` or gets the
766766
# catalog entry.
767767
#
768-
# imageFactoryURL is a plain http(s) URL with no user name or password in it, and
769-
# the platform refuses anything else even when the catalog is off: it is copied into
770-
# every tenant namespace and into the spec of every new pool.
768+
# imageFactoryURL is a plain http(s) URL with no user name, password, query or
769+
# fragment in it, and the platform refuses anything else even when the catalog is
770+
# off: it is copied into every tenant namespace and into the spec of every new pool.
771771
#
772772
# A field the spec already carries, empty or not, is never overwritten, so a pool
773773
# can opt out with `osImage.factory: {}`. Only creation is affected: a pool that

0 commit comments

Comments
 (0)