Skip to content

Conversation

@Szarny
Copy link
Contributor

@Szarny Szarny commented Jun 10, 2022

What this PR does / why we need it:

Add breaking changes note about Helm values file restriction (#3726).

This is because there was an inquiry regarding the path that can be specified as a values file, and we felt it necessary to describe the specifications in more detail.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

@Szarny Szarny self-assigned this Jun 10, 2022
# allowed
- values.yaml
- ./foo/bar/values.yaml
- /path/to/dir-where-application-configuration-exists/values.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- /path/to/dir-where-application-configuration-exists/values.yaml
- /path/to/application-configuration-dir/values.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewrite it with disallowed examples 👍

- /path/to/dir-where-application-configuration-exists/values.yaml

# disallowed
- ../../../../path/to/dir-where-application-configuration-file-NOT-exists/values.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- ../../../../path/to/dir-where-application-configuration-file-NOT-exists/values.yaml
- ../../../../path/to/OTHER-application-dir-or-such/values.yaml


# disallowed
- ../../../../path/to/dir-where-application-configuration-file-NOT-exists/values.yaml
- /path/to/dir-where-application-configuration-NOT-exists/values.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- /path/to/dir-where-application-configuration-NOT-exists/values.yaml
- /path/to/OTHER-application-dir-or-such/values.yaml


According to a recently discovered [vulnerability](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24348), it reveals that the existence of a directory traversal vulnerability when an arbitrary path can be specified as the Helm values file path.

For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration (i.e. `.pipecd.yaml`) exists when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration (i.e. `.pipecd.yaml`) exists when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).
For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration exists (aka. the application directory) when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).

Tsubasa Umeuchi added 2 commits June 10, 2022 12:50

According to a recently discovered [vulnerability](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-24348), it reveals that the existence of a directory traversal vulnerability when an arbitrary path can be specified as the Helm values file path.

For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration (i.e. `.pipecd.yaml`) exists when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration (i.e. `.pipecd.yaml`) exists when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).
For this reason, PipeCD has restricted the path that can be specified as the values file path to the directory where the application configuration (i.e. `app.pipecd.yaml`) exists when a local path is specified by [#3726](https://github.com/pipe-cd/pipecd/pull/3726).

Copy link
Member

@knanao knanao Jun 10, 2022

Choose a reason for hiding this comment

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

Oops, this is better.
so let's merge this one.

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

👍 Thank you

@Szarny
Copy link
Contributor Author

Szarny commented Jun 10, 2022

Which should I adopt?

#3744 (comment)
#3744 (comment)

@Szarny Szarny enabled auto-merge (squash) June 10, 2022 03:53
Copy link
Member

@knanao knanao left a comment

Choose a reason for hiding this comment

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

Here you go!

@Szarny Szarny merged commit ddf240a into pipe-cd:master Jun 10, 2022
@github-actions github-actions bot mentioned this pull request Jul 5, 2022
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.

3 participants