Oops - done. Thanks, @gkurz!

@burgerdev , I think this is a good change - thank you!

In addition to this change: did you also consider setting up the Write side of the pipe for non-blocking Writes? If that works, it should help a CoCo Guest work a little better if its untrusted Host decides to stop calling ReadStream.

You mean setting O_NONBLOCK on the write end and passing that to the container process as stdout (stderr)? This will return EAGAIN for writes that would be blocking otherwise, and I fear that most processes will not handle this situation gracefully. For example, this bash program run with non-blocking pipe that is not cleared fails:

set -e

report() {
    ret=$?
    printf "shell exit code: %d\n" "$ret" >&2
    exit "$ret"
}

trap report EXIT

while true; do
  echo foo
done

/bin/sh: line 12: echo: write error: Resource temporarily unavailable
shell exit code: 1

In addition to this change: did you also consider setting up the Write side of the pipe for non-blocking Writes? If that works, it should help a CoCo Guest work a little better if its untrusted Host decides to stop calling ReadStream.

As well explained by @burgerdev , O_NONBLOCK won't help. If DoS is a concern, the agent should monitor the read end of the pipe, consume the data and discard it. ReadStream should just always return an empty response in this case.

With respect to confidential computing, DoS is usually out of scope because the infrastructure provider can always decide not to run the workload (cf. https://www.redhat.com/en/blog/confidential-computing-primer).

With respect to confidential computing, DoS is usually out of scope because the infrastructure provider can always decide not to run the workload (cf. https://www.redhat.com/en/blog/confidential-computing-primer).

This is my understanding as well. I was just commenting on @danmihai1's suggestion.

@burgerdev Do you think you could also add a test case for this?

@burgerdev , I guess rebasing on the latest main code might help with this test failure: https://github.com/kata-containers/kata-containers/actions/runs/13076857575/job/36496222189?pr=10818 .

(That's how we got that test to pass in #10811)

I rebased and fixed the clippy warning.

@burgerdev Do you think you could also add a test case for this?

A simple test would be to make the existing k8s-policy-job.yaml write more output than the pipe buffer can handle. Alternatively, we could set up a full test that also tests that logs are allowed if configured, etc. What do you think?

I rebased and fixed the clippy warning.

@burgerdev Do you think you could also add a test case for this?

A simple test would be to make the existing k8s-policy-job.yaml write more output than the pipe buffer can handle. Alternatively, we could set up a full test that also tests that logs are allowed if configured, etc. What do you think?

You could also have unit testing of read_stdout and read_stderr actionable by cargo test. I'd favor that if possible (and I suspect it is).

I rebased and fixed the clippy warning.

@burgerdev Do you think you could also add a test case for this?

A simple test would be to make the existing k8s-policy-job.yaml write more output than the pipe buffer can handle. Alternatively, we could set up a full test that also tests that logs are allowed if configured, etc. What do you think?

You could also have unit testing of read_stdout and read_stderr actionable by cargo test. I'd favor that if possible (and I suspect it is).

I like the idea of unit testing this, but struggle to implement it. The closest example I see is test_do_write_stream, but it looks intimidatingly complicated for the simple API surface it covers. Also, it does not look like there's an existing unit test that uses policy, and I fear it's going to be even more complicated because of the global state. :/

@burgerdev You can take inspiration here on how to use the policy in an e2e test - other test cases also use a very similar approach

bd6eedc#diff-8aefc3b3747a8e1e0fbc75e5ee84b20b7ccb4853a144c9102ea1999b4d2c3041

I like the idea of unit testing this, but struggle to implement it. The closest example I see is test_do_write_stream, but it looks intimidatingly complicated for the simple API surface it covers.

I'm not really convinced by the intimidatingly complicated argument 😉

Also, it does not look like there's an existing unit test that uses policy, and I fear it's going to be even more complicated because of the global state. :/

Another suggestion is to add an is_allowed bool argument to do_read_stream and do the resp.clear_data() there. This would consolidate the logic in one place and you should be able to unit test that with not that many lines of code IMO.

Thanks for adding the test @sprt!