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:00:04 UTC
[PR] Apply worker file ownership and mode via an open descriptor (storm)
reiabreu opened a new pull request, #9006:
URL: https://github.com/apache/storm/pull/9006
setup_permissions() opens each entry with O_NOFOLLOW, confirms via fstat that it is still the same object fts_read() classified (matching device and inode), and uses fchown/fchmod on that descriptor instead of lchown/chmod by pathname.
## How this was tested
Built the worker-launcher native tree with `autoreconf -i && ./configure && make check` (autotools, compiled with `-Werror`); the test suite passes.
--
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] Apply worker file ownership and mode via an open descriptor (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 merged PR #9006:
URL: https://github.com/apache/storm/pull/9006
--
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] Apply worker file ownership and mode via an open descriptor (storm)
Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9006:
URL: https://github.com/apache/storm/pull/9006#issuecomment-5394776947
The follow-up test coverage landed in #9014 (OCI launch-command enforcement). The `setup_permissions` fd/dev-ino branch noted here is not included: its privileged walk runs under `seteuid(0)` and needs root, which CI's `make check` does not run as, so only an artificial decision-helper could be unit-tested without root.
--
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] Apply worker file ownership and mode via an open descriptor (storm)
Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9006:
URL: https://github.com/apache/storm/pull/9006#issuecomment-5393489279
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 (O_NONBLOCK):** Added `O_NONBLOCK` to the open flags in `setup_permissions` — no-op for regular files/dirs, `fchown`/`fchmod` on the fd still work, and the `fstat` dev/ino check now rejects a file swapped for a writerless FIFO instead of blocking the root launcher.
- **2 (tests):** Not covered here — `setup_dir_permissions` calls `seteuid(0)` and `setup_permissions` is `static`, so the open/`fstat`-failure and dev/ino-mismatch branches need the function de-static'd or a test wrapper; left for a separate 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] Apply worker file ownership and mode via an open descriptor (storm)
Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on PR #9006:
URL: https://github.com/apache/storm/pull/9006#issuecomment-5393728822
Following up on the test-coverage point: exercising the `setup_permissions` branches needs the function de-static'd, and the full privileged walk needs root (which CI's `make check` doesn't run as). I'll fold what's unit-testable — the dev/ino decision — into the same worker-launcher test-coverage follow-up PR once these PRs are on `master`, alongside the `main.c` enforcement tests noted on #9008 and #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