Skip to content

Commit

Permalink
vdk-audit: [bug fix] Fix incorrectly detected event (#2548)
Browse files Browse the repository at this point in the history
The logic in the `_audit()` method filters the forbidden events and
detects if an executed event is allowed or not. However, the way it
matches the events is by partial string match (the `event in
not_permitted_event` part). This means that if a forbidden event is,
e.g., `os.removexattr` and the event that is passed to the method is
`os.remove`, there will be a match, because `os.remove` is a sub-string
of `os.removexattr`.

With this change, the `_audit()` method switches to exact string match
when filtering for forbidden events, in order to avoid situations where
unexpected events are matched.

Testing done: Added test

Signed-off-by: Andon Andonov <[email protected]>
  • Loading branch information
doks5 authored Aug 10, 2023
1 parent af3f612 commit d2ef7d9
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def initialize_job(self, context: JobContext) -> None:

def _audit(event, args):
if any(
event in not_permitted_event
event == not_permitted_event
for not_permitted_event in forbidden_events_list
):
logger.warning(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright 2021-2023 VMware, Inc.
# SPDX-License-Identifier: Apache-2.0
import os

from vdk.api.job_input import IJobInput


def run(job_input: IJobInput):
with open("test_file.txt", "w") as f:
f.write("Some string")

os.remove("test_file.txt")
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,23 @@ def test_audit_multiple_events_enabled_and_permitted_action():
print(result.output)
cli_assert_equal(0, result)
assert not os._exit.called


def test_audit_single_event_enabled_and_not_misclassified_error():
with mock.patch.dict(
os.environ,
{
"VDK_AUDIT_HOOK_ENABLED": "True",
"VDK_AUDIT_HOOK_FORBIDDEN_EVENTS_LIST": "os.removexattr;",
},
):
os._exit = mock.MagicMock()
runner = CliEntryBasedTestRunner(audit_plugin)

result: Result = runner.invoke(
["run", jobs_path_from_caller_directory("os-remove-command-job")]
)

print(result.output)
cli_assert_equal(0, result)
assert not os._exit.called

0 comments on commit d2ef7d9

Please sign in to comment.