Skip to content
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.

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

Ensure the dashboard zip is sane #6921

Merged
merged 3 commits into from
May 2, 2018
Merged

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Apr 23, 2018

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.
@tsg tsg requested a review from urso April 23, 2018 14:10
return err
}
if strings.HasPrefix(filepath.ToSlash(relPath), "../") {
return fmt.Errorf("Zip file contains files outside of directory target: %s", relPath)
Copy link

Choose a reason for hiding this comment

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

s/directory target/target directory/ ?

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. A test with an invalid zip would have been nice.

@hansmi
Copy link

hansmi commented Apr 24, 2018

If I may I'd recommend using cyphar/filepath-securejoin rather than coming up with your own path verification.

@tsg tsg added the needs_backport PR is waiting to be backported to other branches. label May 2, 2018
tsg added a commit to tsg/beats that referenced this pull request May 2, 2018
This uses the same approach and code as the Kibana dashboards
loader.

Fixes elastic#6921.
@urso urso merged commit a34c9eb into elastic:master May 2, 2018
tsg added a commit to tsg/beats that referenced this pull request May 2, 2018
* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit a34c9eb)
@tsg tsg added v6.3.0 and removed needs_backport PR is waiting to be backported to other branches. labels May 2, 2018
tsg added a commit to tsg/beats that referenced this pull request May 2, 2018
* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit a34c9eb)
@tsg tsg added the v6.2.5 label May 2, 2018
urso pushed a commit that referenced this pull request May 2, 2018
This uses the same approach and code as the Kibana dashboards
loader.

Fixes #6921.
urso pushed a commit that referenced this pull request May 2, 2018
* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit a34c9eb)
tsg added a commit to tsg/beats that referenced this pull request May 2, 2018
This uses the same approach and code as the Kibana dashboards
loader.

Fixes elastic#6921.

(cherry picked from commit a7c9062)
andrewkroh pushed a commit that referenced this pull request May 2, 2018
This uses the same approach and code as the Kibana dashboards
loader.

Fixes #6921.

(cherry picked from commit a7c9062)
@tsg
Copy link
Contributor Author

tsg commented May 3, 2018

Thank you for the suggestion @hansmi. We had some concerns about the Windows portability of the filepath-securejoin lib, because it relies on the PathSeparator. So for now we merged the PR like this to catch the release train, but we'll follow up on this. cc @urso

urso pushed a commit that referenced this pull request May 3, 2018
* Ensure the dashboard zip is sane (#6921)

* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit a34c9eb)

* cleanup changelog
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit 028a3fd)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…#6996)

This uses the same approach and code as the Kibana dashboards
loader.

Fixes elastic#6921.

(cherry picked from commit b514dd1)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…astic#6995)

* Ensure the dashboard zip is sane (elastic#6921)

* Ensure the dashboard zip is sane

This adds a check that all files from the dashboard zip file
are pointing to the right target, and don't override other configs.

* changelog

* addressed comment

(cherry picked from commit 028a3fd)

* cleanup changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants