Validate the container id and invoke runc without a shell in reap-oci-container

[reiabreu]

reap-oci-container now checks the container id with validate_container_id (hex digits and dashes, 36-42 chars) before it is used. validate_container_id is exported from oci_launch_cmd.c. cleanup_oci_container runs "runc delete" via fork/execv with an explicit argument vector instead of system().

How this was tested

Built the worker-launcher native tree with autoreconf -i && ./configure && make check (autotools, compiled with -Werror); the test suite passes. Adds test_validate_container_id.

[rzo1]

Currently low on time, thus a LLM based review.

Direction is good (dropping the shell is the right call). Two minor points:

1. A validated container id can still begin with -, and it's passed to runc delete without a -- separator (minor).
validate_container_id checks only length (36–42) and charset (all_uuid_digit = hex + -), with no positional/UUID-structure check, so an all-dash or leading-- string passes. In cleanup_oci_container the child execs { runc_path, "delete", container_id, NULL } — runc's CLI parser then treats a leading-dash id as a flag rather than the positional argument. No harmful flag is spellable in [0-9a-fA-F-] at that length, so it fails safe (misparsed/failed delete, rc=1) rather than escalating, but it doesn't fully deliver the "safe on the runc command line" goal. Inserting -- before container_id, or rejecting ids whose first char is -, closes the gap. (Same option-shaped ids also reach docker inspect ... <worker_id>, which likewise fails safely to pid=-1.)

2. Tests cover only the validator, not the branches the fix adds (minor).
test_validate_container_id() exercises the validator in isolation, but the substance of the PR — the new rejection branches in main.c and replacing system() with fork/execv in cleanup_oci_container — isn't tested: main.c isn't linked into the test binary and no test invokes cleanup_oci_container, so reverting either would still pass the whole suite. Minor nit: the "6702-x nope; other stuff" case is 24 chars and is rejected by the length check, not the character check — only the slash case actually exercises all_uuid_digit.

[reiabreu]

Thank you for the feedback, @rzo1. I'm afraid I haven't touched C in many years, so I relied entirely on Claude for these fixes.

• 1 (--): cleanup_oci_container now execs runc delete -- <id>, so an option-shaped id can't be parsed as a flag.
• 2 (test nit / coverage): Replaced the 24-char case (rejected by the length check) with a 40-char invalid-charset case that actually exercises the character check. cleanup_oci_container and the main.c rejection branch still aren't run because main.c isn't linked into the test binary — left for a separate change.

[reiabreu]

Following up on the test-coverage point: cleanup_oci_container and the main.c reject branch aren't run because main.c isn't linked into the test binary. I'll address this in a single worker-launcher test-coverage follow-up PR once these PRs are on master (extracting the dispatch logic into a test-linked source), together with the parse-level tests noted on #9010.

[reiabreu]

The follow-up test coverage landed in #9014. It establishes the pattern of extracting main.c logic into a test-linked helper (for the launch-command username check) and adds a parse-level test. cleanup_oci_container and the main.c container-id reject branch remain uncovered there — validate_container_id itself is already tested, but exercising the reject/reap paths needs main.c linked and a runc stub, which I've left out for now.