fix: make install robust under umask 077 (test + MTProxy binary perms) - #9
fix: make install robust under umask 077 (test + MTProxy binary perms)#9bip-bup-bip-bup wants to merge 2 commits into
Conversation
install.sh sets umask 077, so os.WriteFile's requested 0444 mode is silently reduced to 0400 and the 'group/other-readable profiles file outside a credential directory' assertion never fires. Chmod the file explicitly so the test verifies the intended permission handling under any umask.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a39efccf5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # install.sh runs with umask 077, so make's output keeps owner-only | ||
| # permissions; the unprivileged mtproxy service user must be able to | ||
| # traverse the tree and execute the binary. | ||
| chmod -R a+rX "$build_directory" |
There was a problem hiding this comment.
Normalize permissions for reused MTProxy installs
When rerunning the installer after a previous failed run under umask 077, /opt/MTProxy can already contain the pinned commit with a 0700 root-owned objs/bin/mtproto-proxy and 0600 .tproxy-commit; because the guard above is evaluated as root, -x and grep still succeed, this rebuild block is skipped, and the new chmod never runs. In that recovery path mtproxy.service still cannot traverse/execute the existing tree as the mtproxy user, so move the permission normalization to also cover the reused $source_directory or check executability as mtproxy.
Useful? React with 👍 / 👎.
install.sh runs with umask 077, so make's output keeps owner-only
permissions. After chown root:root the binary and its directories are
0700 root, and mtproxy.service (User=mtproxy) fails with 203/EXEC
because it cannot traverse the tree or execute the binary, leaving
tproxy-server /readyz stuck at 503 ('tproxy-server did not become
ready').
Normalize ownership and read/execute bits unconditionally after the
build guard: a previous failed run may have left /opt/MTProxy with a
root-owned 0700 binary, and the guard (checked as root) then skips the
rebuild while the tree stays unusable for the mtproxy user. Verified by
simulating the broken state on a live install: binary 0700 -> script
skips rebuild, applies 0755, mtproxy.service comes up, /readyz = 200.
a39efcc to
217ba25
Compare
|
Good catch — thanks! That is exactly the recovery path we hit on a live install (first failed run left /opt/MTProxy with a root-owned 0700 binary; the rebuild guard, checked as root, skipped the rebuild on the second run and the permission fix never applied). Updated the commit: ownership and read/execute bits are now normalized unconditionally after the build guard, so a reused tree gets fixed too. Verified on the live instance by simulating the broken state: chmod -R go-rwx /opt/MTProxy → mtproxy.service crash-looping → re-ran install-mtproxy.sh (rebuild correctly skipped) → binary 0755 → mtproxy.service active → /readyz = 200. |
Summary
deploy/install.shsetsumask 077at the top, which breaks two things on a fresh install:go test ./...fails —TestLoadAcceptsSystemdCredentialReadPermissionsininternal/configwritesprofiles.jsonwithos.WriteFile(..., 0444), but under umask 077 the file is actually created0400. The second assertion ("group/other-readable profiles file outside a credential directory was accepted") then vacuously passes, the test fails, and the installer aborts before installingtproxy-server.MTProxy fails to start with
203/EXEC—install-mtproxy.shbuilds the official MTProxy as themtproxyuser and thenchown -R root:roots the tree, but never adds group/other permissions. Under umask 077 the binary and directories stay0700 root, somtproxy.service(User=mtproxy) cannot traverse/opt/MTProxy/objsor execute the binary.tproxy-serverthen never becomes ready (/readyz= 503) and the installer exits with "tproxy-server did not become ready".Changes
internal/config/config_test.go:os.Chmod(profiles, 0444)after write, so the test exercises the intended permission handling regardless of umask.deploy/install-mtproxy.sh:chmod -R a+rXafterchown -R root:root, so the unprivilegedmtproxyservice user can traverse the tree and execute the binary.Test Plan
Verified on a production Ubuntu 22.04 x86_64 server where the previous install failed at both points:
Also verified end-to-end:
tg://webproxylink works from Telegram Desktop 7.1.1.