Skip to content

OCPNODE-4526: Block '..' traversal in StorePath #2999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saschagrunert wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OCPNODE-4526: Block '..' traversal in StorePath #2999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions ...ts/containerruntimeconfigs.machineconfiguration.openshift.io/AdditionalStorageConfig.yaml
View file
Open in desktop

@JoelSpeed JoelSpeed Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will want a ratcheting test to demonstrate that existing instances of the bad validation don't block writes to adjacent fields in the API

@saschagrunert saschagrunert Aug 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, done in https://github.com/openshift/api/compare/b36f4de40b3a253b50a6c9cce3cae3b3f52c2e82..d729ac6ebd660eaac838606b39d4b3f5e511359c

Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,65 @@ tests:
- path: /mnt/ssd-artifacts
expectedError: "additionalArtifactStores must not contain duplicate paths"

# Path traversal tests
- name: Should fail if additionalLayerStores path contains dot-dot traversal
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalLayerStores:
- path: /var/lib/../../etc
expectedError: "path must not contain '..' components"

- name: Should fail if additionalImageStores path contains dot-dot traversal
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalImageStores:
- path: /var/lib/../secrets
expectedError: "path must not contain '..' components"

- name: Should fail if additionalArtifactStores path contains dot-dot traversal
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalArtifactStores:
- path: /mnt/store/..
expectedError: "path must not contain '..' components"

# Regression: double dots within a filename are valid (not a traversal component)
- name: Should succeed if path contains double dots within a filename
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalLayerStores:
- path: /var/lib/foo..bar
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalLayerStores:
- path: /var/lib/foo..bar

# Colon in path tests (prevented by regex)
- name: Should fail if additionalLayerStores path contains a colon
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
additionalLayerStores:
- path: /var/lib/store:ref
expectedError: "path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'"

# Combined test - all storage types together with other fields
- name: Should be able to create ContainerRuntimeConfig with all storage types and existing fields
initial: |
Expand Down Expand Up @@ -276,3 +335,89 @@ tests:
- path: /mnt/ssd-images
additionalArtifactStores:
- path: /mnt/ssd-artifacts
onUpdate:
# Ratcheting tests: existing instances with '..' traversal paths must not block updates to adjacent fields
- name: Should be able to update logLevel when additionalLayerStores has an existing path with dot-dot traversal
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/containerRuntimeConfig/properties/additionalLayerStores/items/properties/path/x-kubernetes-validations/2
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: info
additionalLayerStores:
- path: /var/lib/../../etc
updated: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalLayerStores:
- path: /var/lib/../../etc
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalLayerStores:
- path: /var/lib/../../etc
- name: Should be able to update logLevel when additionalImageStores has an existing path with dot-dot traversal
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/containerRuntimeConfig/properties/additionalImageStores/items/properties/path/x-kubernetes-validations/2
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: info
additionalImageStores:
- path: /var/lib/../secrets
updated: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalImageStores:
- path: /var/lib/../secrets
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalImageStores:
- path: /var/lib/../secrets
- name: Should be able to update logLevel when additionalArtifactStores has an existing path with dot-dot traversal
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/containerRuntimeConfig/properties/additionalArtifactStores/items/properties/path/x-kubernetes-validations/2
initial: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: info
additionalArtifactStores:
- path: /mnt/store/..
updated: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalArtifactStores:
- path: /mnt/store/..
expected: |
apiVersion: machineconfiguration.openshift.io/v1
kind: ContainerRuntimeConfig
spec:
containerRuntimeConfig:
logLevel: debug
additionalArtifactStores:
- path: /mnt/store/..
10 changes: 6 additions & 4 deletions machineconfiguration/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,11 +1035,13 @@ const (

// StorePath is an absolute filesystem path used by additional container storage configurations.
// The path must be between 1 and 256 characters long, begin with a forward slash, and only contain
// the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'. Consecutive forward slashes are not permitted.
// the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'. Consecutive forward slashes and '..'
// directory traversal components are not permitted.
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=256
// +kubebuilder:validation:XValidation:rule="self.matches('^/[a-zA-Z0-9/._-]+$')",message="path must be absolute and contain only alphanumeric characters, '/', '.', '_', and '-'"
// +kubebuilder:validation:XValidation:rule="!self.contains('//')",message="path must not contain consecutive forward slashes"
// +kubebuilder:validation:XValidation:rule="self.split('/').filter(s, s == '..').size() == 0",message="path must not contain '..' components"
type StorePath string

// AdditionalLayerStore defines a read-only storage location for Open Container Initiative (OCI) container image layers.
Expand All @@ -1050,7 +1052,7 @@ type AdditionalLayerStore struct {
// retrieving from the registry.
// The path is required and must be between 1 and 256 characters long, begin with a forward slash,
// and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
// Consecutive forward slashes are not permitted.
// Consecutive forward slashes and '..' directory traversal components are not permitted.
// +required
Path StorePath `json:"path,omitempty"`
}
Expand All @@ -1063,7 +1065,7 @@ type AdditionalImageStore struct {
// retrieving from the registry.
// The path is required and must be between 1 and 256 characters long, begin with a forward slash,
// and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
// Consecutive forward slashes are not permitted.
// Consecutive forward slashes and '..' directory traversal components are not permitted.
// +required
Path StorePath `json:"path,omitempty"`
}
Expand All @@ -1076,7 +1078,7 @@ type AdditionalArtifactStore struct {
// retrieving from the registry.
// The path is required and must be between 1 and 256 characters long, begin with a forward slash,
// and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
// Consecutive forward slashes are not permitted.
// Consecutive forward slashes and '..' directory traversal components are not permitted.
// +required
Path StorePath `json:"path,omitempty"`
}
Expand Down
12 changes: 9 additions & 3 deletions ....crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -86,6 +86,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -120,7 +122,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -130,6 +132,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -164,7 +168,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -174,6 +178,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down
12 changes: 9 additions & 3 deletions ...enerated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -86,6 +86,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -120,7 +122,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -130,6 +132,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -164,7 +168,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -174,6 +178,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down
12 changes: 9 additions & 3 deletions ...-manifests/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -86,6 +86,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -120,7 +122,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -130,6 +132,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down Expand Up @@ -164,7 +168,7 @@ spec:
retrieving from the registry.
The path is required and must be between 1 and 256 characters long, begin with a forward slash,
and only contain the characters a-z, A-Z, 0-9, '/', '.', '_', and '-'.
Consecutive forward slashes are not permitted.
Consecutive forward slashes and '..' directory traversal components are not permitted.
maxLength: 256
minLength: 1
type: string
Expand All @@ -174,6 +178,8 @@ spec:
rule: self.matches('^/[a-zA-Z0-9/._-]+$')
- message: path must not contain consecutive forward slashes
rule: '!self.contains(''//'')'
- message: path must not contain '..' components
rule: self.split('/').filter(s, s == '..').size() == 0
required:
- path
type: object
Expand Down
Loading