You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@storm.apache.org by "reiabreu (via GitHub)" <gi...@apache.org> on 2026/08/23 18:03:27 UTC

[PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

reiabreu opened a new pull request, #9010:
URL: https://github.com/apache/storm/pull/9010

   run-oci-container parses the launch command file before setup_dir_permissions changes the worker directory's ownership, and requires the command file's username to match the user passed to the worker-launcher. run_oci_container now takes the parsed oci_launch_cmd, which main owns and frees. Bind-mount sources must be absolute paths with no "." or ".." component, equal to or under a directory listed in the new `worker.launcher.oci.allowed.mount.source.dirs` config key; if none are configured, all sources are rejected.
   
   ## 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_mount_path_helpers` and `test_mount_source_allowed_dirs`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9010:
URL: https://github.com/apache/storm/pull/9010#issuecomment-5393729422

   Following up on the test-coverage point: the enforcement paths in `main.c` (the launch-command username check and the container-id reject dispatch) can't be exercised today because `main.c` isn't linked into the test binary. I'll cover this in a dedicated follow-up PR once this and the related worker-launcher PRs land on `master` — it will pull that dispatch logic into a test-linked source and add a `parse_oci_launch_cmd`-level test (mount-rejection + username-mismatch), rather than stacking a refactor on the still-open branches.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9010:
URL: https://github.com/apache/storm/pull/9010#issuecomment-5393489794

   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 (symlink bypass):** `is_valid_mount_source` now resolves the source and each configured directory with `realpath` before the containment check and rejects unresolvable sources. Docs drop the `/data/storm` example and state that only directories the topology user cannot write to should be listed (runc re-resolves at mount time — residual TOCTOU). Added a symlink-escape test.
   - **2 (degenerate config values):** Split into #9013, since the helper bug affects every `get_values` caller. There `extract_values_delim` NUL-terminates the zero-token case and `get_values` returns independently-owned strings so `free_values` no longer frees an interior pointer; includes a regression test that aborts on the pre-fix code.
   - **3 (`/` allowed dir):** Documented that `/` isn't a usable entry (a path is accepted only when it equals or sits under a listed directory as a whole component).
   - **4 (enforcement-point tests):** The `parse_oci_launch_cmd`-level and username-mismatch tests need `main.c` linked into the test binary — same deferred test-harness change.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9010:
URL: https://github.com/apache/storm/pull/9010#issuecomment-5394403193

   @rzo1 Done — rebased onto `master` (now includes #9006 and #9008). Resolved the overlaps in `oci_launch_cmd.h` and `test-worker-launcher.c` (kept `validate_container_id` alongside the mount-source declarations/tests); `make check` is green under `-Werror`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9010:
URL: https://github.com/apache/storm/pull/9010#issuecomment-5394777462

   The follow-up test coverage is now up in #9014 — it adds the parse-level mount-source rejection test and the username-mismatch test noted in point 4 (the mount test parses a full launch command that is valid except for its bind-mount source, so it exercises the `is_valid_mount` hookup, not just `is_valid_mount_source`).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 merged PR #9010:
URL: https://github.com/apache/storm/pull/9010


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Restrict OCI bind-mount sources to configured directories and check the launch command username (storm)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #9010:
URL: https://github.com/apache/storm/pull/9010#issuecomment-5394262075

   @reiabreu Can you rebase?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org