Bug 987155 - Manifest#initialize should not be calling File.exists? on string contents of downloaded manifests
Manifest#initialize should not be calling File.exists? on string contents of ...
Product: OpenShift Online
Classification: Red Hat
Component: Containers (Show other bugs)
Unspecified Unspecified
unspecified Severity medium
: ---
: ---
Assigned To: Paul Morie
libra bugs
Depends On:
  Show dependency treegraph
Reported: 2013-07-22 16:32 EDT by Clayton Coleman
Modified: 2015-05-14 19:24 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2013-08-29 08:48:35 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Clayton Coleman 2013-07-22 16:32:16 EDT
openshift-origin-common/models/manifest.rb#initialize is taking manifest as an argument, then calling File.exists? on that argument.  If it returns false, it's assuming that the manifest is a downloaded manifest.

This is bad code for a number of reasons (large manifests could raise spurious errors against some filesystems).  Instead, #initialize should take a YAML document as "manifest", and the path as a second argument.  If callers need to parse YAML, they should call a static method on Manifest that handles that behavior:

   def self.load_from_file(path)
     new(YAML.load_file(manifest), path, ...)

Also, the loading code that loads the manifest (@manifest = YAML.load_file(manifest)) is not using safe, which means that it's likely someone could use this incorrectly and expose a security issue.

Medium because of the potential for problems.
Comment 2 Meng Bo 2013-08-09 00:12:51 EDT
Checked on devenv_3632, the change has been merged.

And create app form downloadable cartridge still works.

Move the bug to verified.

Note You need to log in before you can comment on or make changes to this bug.