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(checks): handle file: and multi: in AVD-DS-005 #60

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

candrews
Copy link
Contributor

The reverse engineered Dockerfile of an image doesn't exactly match the original Dockerfile. For example, it doesn't have the original source files names. Instead, it uses file:<hash> in: ADD file:8b8864b3e02a33a579dc216fd51b28a6047bc8eeaa03045b258980fe0cf7fcb3 in /__cacert_entrypoint.sh

Such commands should not trigger AVD-DS-005.

@candrews candrews requested a review from simar7 as a code owner January 17, 2024 19:25
@candrews
Copy link
Contributor Author

@simar7 can you please help me progress this MR?

I'm very eager to eliminate this false positive.

@simar7
Copy link
Member

simar7 commented Feb 2, 2024

@nikpivkin could you take a look?

Comment on lines 28 to 30
cnt := count(copy.Value)
not (cnt == 3 && startswith(add.Value[0], "file:") || startswith(add.Value[0], "multi:") && add.Value[1] == "in")

Copy link
Contributor

@nikpivkin nikpivkin Feb 6, 2024

Choose a reason for hiding this comment

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

I don't have permissions to make changes to this branch.

Rego does not have the && and || operators. All expressions in a rule are evaluated sequentially, which is equivalent to conjunction.

Here is the implementation of your expression in Rego:

Suggested change
cnt := count(copy.Value)
not (cnt == 3 && startswith(add.Value[0], "file:") || startswith(add.Value[0], "multi:") && add.Value[1] == "in")
not is_command_with_hash(add.Value, "file:")
not is_command_with_hash(add.Value, "multi:")
is_command_with_hash(cmd, prefix) {
	count(cmd) == 3
	startswith(cmd[0], prefix)
	cmd[1] == "in"
}

You can also merge main into your branch to run tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

@candrews could you please update this PR as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for providing the ability to run tests, that's very helpful!

I've updated this MR with your suggestion.

The reverse engineered `Dockerfile` of an image doesn't exactly match
the original `Dockerfile`. For example, it doesn't have the original
source files names. Instead, it uses `file:<hash> in`:
`ADD file:8b8864b3e02a33a579dc216fd51b28a6047bc8eeaa03045b258980fe0cf7fcb3 in /__cacert_entrypoint.sh`

Such commands should not trigger AVD-DS-005.
@simar7 simar7 merged commit ecc1ecd into aquasecurity:main Feb 7, 2024
4 checks passed
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