-
Notifications
You must be signed in to change notification settings - Fork 98
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
[WIP]: enhance security with safe-path #23
base: main
Are you sure you want to change the base?
Conversation
The OCI image spec has a section named `Conversion to OCI Runtime Configuration`, which defines how to generate a default runtime configuration from an image. So add more comments to create_runtime_config() by quoting spec definitions. Signed-off-by: Jiang Liu <[email protected]>
Obey the precedence rule defined the OCI image spec when generating annotations for runtime configuration. Signed-off-by: Jiang Liu <[email protected]>
There are three more annotations defined by the OCI image spec, so add support for them. Signed-off-by: Jiang Liu <[email protected]>
Use the safe-path crate to protect from possible path related attacks. Signed-off-by: Jiang Liu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jiangliu - A few comments...
let bundle_config = bundle_path.join(BUNDLE_CONFIG); | ||
spec.save(&bundle_config)?; | ||
|
||
Ok(bundle_config) | ||
PinnedPathBuf::new(bundle_path, BUNDLE_CONFIG) | ||
.context("The path of generated config.json file has changed, possible attack!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Could you use
BUNDLE_CONFIG
rather than hard-codingconfig.json
. - I wonder if we want to use a standard "eye catcher" / tag for scenarios like this (maybe use a "SECURITY" prefix in the context message?) That would make errors like this more
grep(1)
-able.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any update on this?
Ping @jiangliu. |
Re-ping @jiangliu 😄 |
No description provided.