-
Notifications
You must be signed in to change notification settings - Fork 12
feat(cli): validator container runs with cartesi unprivileged user #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(cli): validator container runs with cartesi unprivileged user #152
Conversation
🦋 Changeset detectedLatest commit: 61ab946 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report for ./apps/cli
File Coverage
|
||||||||||||||||||||||||||||||||||||||
78b5056 to
d2c37a4
Compare
Using the post_start docker compose hook, we can create the snapshot directory and copy the snapshot using the root user and start the container with cartesi unprivileged user. Requires compose plugin 2.30.0 See: https://docs.docker.com/compose/how-tos/lifecycle/
49bd116 to
61ab946
Compare
| - | | ||
| mkdir -p "$CARTESI_SNAPSHOT_DIR" | ||
| cp --recursive /tmp/snapshot/* "$CARTESI_SNAPSHOT_DIR" | ||
| while ! stat "$CARTESI_SNAPSHOT_DIR" &>/dev/null; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This busy wait and coordination between the command and post_start is not very elegant.
Does it really need to be this way? What if the post_start fails for whatever reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add some timeout to ensure it fails.
Does it really need to be this way?
It's my approach to run the validator with an unprivileged user leveraging compose.
There are other approaches, for sure, would have to test some alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we first see how the start + deploy would work. Because it will be different in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, we can put this on-hold
if the rollups-node container image release already has this in-place, this process or preparing a directory should just be unnecessary
|
In any case, if we move forward with a system-wide |
a82afdd to
a7e55c5
Compare
39558cf to
d496299
Compare
b999336 to
24ad87e
Compare
|
Is there still a reason for this PR? |
No, since rollups-node doesn't have a container image release anymore. Se could aim to run all services with unorivileged users, on another PR. |
Using the post_start docker compose hook, we can create the snapshot directory and copy the snapshot using the root user and start the container with cartesi unprivileged user.
Requires compose plugin 2.30.0
See: https://docs.docker.com/compose/how-tos/lifecycle/
This pull request includes several updates to the
@cartesi/clipackage, focusing on improving security and updating dependencies. The most important changes include running the validator container with an unprivileged user and updating the minimum required Docker Compose version.Security improvements:
apps/cli/src/node/docker-compose-validator.yaml: Modified theservicessection to run the validator container with thecartesiunprivileged user instead ofroot.Dependency updates:
apps/cli/src/commands/doctor.ts: Updated the minimum required Docker Compose version to2.30.0.Documentation:
.changeset/seven-seals-pull.md: Added a changeset entry to document the update to run the validator container with thecartesiunprivileged user.