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

fix(fix): mixed up change summary list #1185

Merged
merged 1 commit into from Apr 9, 2023
Merged

fix(fix): mixed up change summary list #1185

merged 1 commit into from Apr 9, 2023

Conversation

HollowMan6
Copy link
Contributor

@HollowMan6 HollowMan6 commented Apr 6, 2023

Overview

Fix the mixed up change list (I don't know why you use pointers here, which is actually the culprit):

for _, rulePaths := range rule.Paths {
if rulePaths.FixPath.Path == "" {
continue
}
if strings.HasPrefix(rulePaths.FixPath.Value, UserValuePrefix) && skipUserValues {
continue
}
yamlExpression := fixPathToValidYamlExpression(rulePaths.FixPath.Path, rulePaths.FixPath.Value, documentIndex)
rfi.YamlExpressions[yamlExpression] = &rulePaths.FixPath

rulePaths is a temporary variable, which will get released after use, it would of course be wrong if we store the address of that variable and try to use it after release.

kubescape scan test.yaml --format json --format-version v2 --output results
kubescape fix --no-confirm results.json

Issue before this fix

apiVersion: v1
kind: Pod
metadata:
  name: insert_to_mapping_node_1

spec:
  containers:
  - name: nginx_container
    image: nginx
# Proposed changes:
    securityContext:
      runAsNonRoot: true
      allowPrivilegeEscalation: false
      readOnlyRootFilesystem: true
  securityContext:
    runAsNonRoot: true
    allowPrivilegeEscalation: false

The 3rd one and 5th one are definitely wrong:

[info] The following changes will be applied:
File: test.yaml
Resource: insert_to_mapping_node_1
Kind: Pod
Changes:
	1) spec.containers[0].securityContext.allowPrivilegeEscalation = false
	2) spec.securityContext.allowPrivilegeEscalation = false
	3) spec.securityContext.allowPrivilegeEscalation = false
	4) spec.containers[0].securityContext.readOnlyRootFilesystem = true
	5) spec.containers[0].securityContext.allowPrivilegeEscalation = false

After fix

Now it works:

[info] The following changes will be applied:
File: test.yaml
Resource: insert_to_mapping_node_1
Kind: Pod
Changes:
	1) spec.containers[0].securityContext.readOnlyRootFilesystem = true
	2) spec.containers[0].securityContext.runAsNonRoot = true
	3) spec.containers[0].securityContext.allowPrivilegeEscalation = false
	4) spec.securityContext.runAsNonRoot = true
	5) spec.securityContext.allowPrivilegeEscalation = false

Signed-off-by: Hollow Man <hollowman@opensuse.org>
Copy link
Contributor

@dwertent dwertent 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 :)

@dwertent dwertent merged commit c7af626 into kubescape:master Apr 9, 2023
6 checks passed
@HollowMan6 HollowMan6 deleted the fix-changes branch April 9, 2023 09:20
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.

None yet

2 participants