Repository navigation
[OMEGA-470] Fix: Memory transfer permissions - #377
Conversation
paul-v-snet
left a comment
There was a problem hiding this comment.
A few things I noticed. I may be missing some context, so please correct me where I'm wrong.
su --supp-groupwith a numeric GID
As far as I can tell, util-linux su resolves --supp-group only by name: add_supp_group() calls getgrnam() (su-common.c#L1005-L1017). If so,su --supp-group 1001 nobodywould fail with group 1001 does not exist unless the container's /etc/group already has that group. Since recover always goes throughagent_su, I think the container might exit at startup whenever a transfer dir is configured. When I tried it, even --supp-group 100 was rejected, though users exists there. The preflight probably doesn't catch this because it usesdocker run --group-add, which accepts numeric GIDs. Since the main container runs with -d, the failure would likely only show up in docker logs.
Also, per su(1), the first -G group becomes the primary group unless -g is given, so the agent seems to lose nogroup.- ACL check looks like it's always bypassed
The heredoc haselsewithout a colon, so Python raises aSyntaxErrorand exits with 1. That's treated as "no ACL". On a dir withsetfacl -m u:nobody:rwx, the check passed for me and printed a traceback. The python3 stub in the tests ("-"->exit 1) would hide this. - Not sure the original issue is fully addressed
The installer no longer creates the dir, but it also doesn't seem to validate the path, mode or GID. As far as I can see, all checks still run instart()at the end of the wizard, after docker pull. So I think a misprepared dir would still fail at the same point, just with a different message. Maybe the installer could run the same checks up front, default the GID to the dir'sst_gid, and point to the setup docs? - The
uname != Linuxcheck seems to turn the feature off on macOS / Docker Desktop, where bind mounts might have worked before. Is that intended?
Co-authored-by: paul-v-snet <paul.vasiletz@singularitynet.io>
…-permissions # Conflicts: # tests/test_omegaclaw_launcher.py
|
2,5 cents from me: For the release, I suggest the fix should:
From @paul-v-snet's review, these should be solved before testing: the numeric GID in Running the agent as the launching user instead of This answer was prepared with the help of my AI assistant. |
timur-ashkenov
left a comment
There was a problem hiding this comment.
Addressed the review feedback in this branch:
- fixed the numeric
MEMORY_TRANSFER_GIDhandling inentrypoint.sh: the numeric GID is resolved or created as a group name before it is passed tosu --supp-group;nogroupremains the agent’s primary group; - kept the Linux memory-transfer directory model private and group-based, without
0777; - moved directory validation to the installer flow, before Docker image pulling, and let the user skip memory transfer if the directory is not prepared;
- added a warning before an overwrite-mode memory import;
- added coverage for the installer validation, Linux-only contract, preflight setup, and import warning.
I also tested the proposed entrypoint.sh ownership/mode workaround on macOS Docker. A simple write can pass, but the actual memory export still fails under Omega’s Landlock policy. Therefore the secure bind-mount memory-transfer flow remains supported on native Linux only for now.
paul-v-snet
left a comment
There was a problem hiding this comment.
Please check and fix it if that is the case
|
Good catch. This was not caught because my local testing ran from a repository checkout, where |
|
Tested: 9025f74 and an image built from it on a Linux host. The launcher ran as a regular user in the The agent does not start with a group the image does not haveentrypoint.sh:46 looks the GID up with The setup steps in docs/reference-memory-portability.md create a new host group, and the image does not have it. I followed those steps with GID 4470, and the agent never started. GID 100 ( With What I checked
Smaller things
Verdict: FAIL |
|
Tested: e82ea6f and an image built from it on a Linux host. The launcher ran as a regular user in the What I checked
Verdict: PASS |
|
Can you approve this PR please? |
Description
Fixes memory-transfer startup failures without making archive directories world-writable.
Memory transfer now requires a pre-created Linux directory with a private group, setgid mode 2770, and no ACL access. The launcher validates ownership, mode, GID, symlinks, and ACL state before startup. The container entrypoint remains root for Nginx/bootstrap, while only nobody agent/import/recovery processes receive the configured supplementary transfer group.
Adds --memory-transfer-gid and updates the wizard and documentation with the required Linux setup.
How Has This Been Tested?
Result: 78 passed, 1 skipped on macOS.
Result: 7 passed, 1 skipped.
The Linux Docker integration test is skipped on macOS by design because Docker Desktop does not provide native Linux host UID/GID/ACL semantics.
Checklist