Skip to content

fix: make install robust under umask 077 (test + MTProxy binary perms) - #9

Open
bip-bup-bip-bup wants to merge 2 commits into
telegramdesktop:masterfrom
bip-bup-bip-bup:fix/umask-077-install
Open

fix: make install robust under umask 077 (test + MTProxy binary perms)#9
bip-bup-bip-bup wants to merge 2 commits into
telegramdesktop:masterfrom
bip-bup-bip-bup:fix/umask-077-install

Conversation

@bip-bup-bip-bup

Copy link
Copy Markdown

Summary

deploy/install.sh sets umask 077 at the top, which breaks two things on a fresh install:

  1. go test ./... failsTestLoadAcceptsSystemdCredentialReadPermissions in internal/config writes profiles.json with os.WriteFile(..., 0444), but under umask 077 the file is actually created 0400. 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 installing tproxy-server.

  2. MTProxy fails to start with 203/EXECinstall-mtproxy.sh builds the official MTProxy as the mtproxy user and then chown -R root:roots the tree, but never adds group/other permissions. Under umask 077 the binary and directories stay 0700 root, so mtproxy.service (User=mtproxy) cannot traverse /opt/MTProxy/objs or execute the binary. tproxy-server then 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+rX after chown -R root:root, so the unprivileged mtproxy service 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:

umask 077 && go test ./... -count=1   # all pass
# after fix: mtproxy active, /readyz = 200, install completes

Also verified end-to-end: tg://webproxy link works from Telegram Desktop 7.1.1.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread deploy/install-mtproxy.sh Outdated
# 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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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.
@bip-bup-bip-bup

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant