fix: harden Railway startup persistence and readiness - #224
Coding-Dev-Tools wants to merge 13 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e085f312c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aff0a37fd1
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e52158582
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bc085a56c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0c77f5599
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a32f1d005
ℹ️ 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".
| elif [ ! -f "$ownership_marker" ]; then | ||
| printf '%s\n' "[engraphis] refusing non-regular volume ownership marker: $ownership_marker" >&2 | ||
| exit 1 | ||
| elif ! chown engraphis:engraphis /data "$state_dir" "$ownership_marker"; then |
There was a problem hiding this comment.
Refuse chowning arbitrary existing state directories
When ENGRAPHIS_STATE_DIR names an existing root-owned directory outside /data (for example, /usr/local/bin), the first boot accepts it and creates/chowns only .volume-ownership; on the next boot this branch transfers ownership of the directory itself to engraphis. A compromised runtime can then replace /usr/local/bin/docker-entrypoint.sh, which root executes on the following restart. The current tree's new symlink/non-directory checks are fresh evidence that this remains reachable because a regular root-owned directory passes them; constrain the state directory to /data or require an external directory to already be app-owned instead of chowning it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85390881a6
ℹ️ 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".
| # ENGRAPHIS_STATE_DIR defaults to /data/.engraphis. Repair the complete volume only on | ||
| # first boot; later restarts verify the mount and state roots without walking the cache. | ||
| state_dir="${ENGRAPHIS_STATE_DIR:-/data/.engraphis}" | ||
| ownership_marker="${state_dir}/.volume-ownership" |
There was a problem hiding this comment.
Keep the ownership marker on the managed /data volume
When the supported ENGRAPHIS_STATE_DIR override points to a separately persistent external directory, this marker tracks that directory rather than the /data volume whose recursive ownership repair it controls. If the external state is reused while /data is replaced or restored with root-owned database/cache files, the stale marker skips chown -R; the later non-recursive chown changes only /data itself, so the app user can fail to open the existing files after gosu. Store the marker on /data or otherwise bind it to the actual data volume being repaired.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f65fee2c51
ℹ️ 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".
| ENGRAPHIS_STATE_DIR=/data/.engraphis | ||
| ENGRAPHIS_STATE_DIR=/data/.engraphis \ | ||
| # Dashboard-managed non-secret settings must survive a Railway redeploy with the volume. | ||
| ENGRAPHIS_ENV_FILE=/data/.engraphis/config.env |
There was a problem hiding this comment.
Allow first boot when the container starts non-root
When the image is run with --user 10001 or a Kubernetes runAsUser and /data/.engraphis/config.env has not already been provisioned, the root-only entrypoint block never creates this newly explicit config file. Because config._load_trusted_dotenv() treats an explicit ENGRAPHIS_ENV_FILE as required (allow_missing=False), importing the configuration raises FileNotFoundError and the dashboard exits before serving; ensure the non-root path creates the file when writable or tolerates its initial absence.
Useful? React with 👍 / 👎.
| printf '%s\n' "[engraphis] external state directory must already be owned by engraphis: $state_dir" >&2 | ||
| exit 1 | ||
| fi | ||
| if ! mkdir -p "$state_dir"; then |
There was a problem hiding this comment.
Create the state directory with private permissions
On a fresh /data volume under the image's normal 022 umask, this creates /data/.engraphis as mode 0755, and the later chown -R changes only ownership. The directory holds the trusted config and cloud-session files, so another UID sharing the volume can enumerate their names and metadata until a later settings/session write happens to harden the directory; create or chmod it as 0700 before dropping privileges.
AGENTS.md reference: AGENTS.md:L48-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8bd797e59
ℹ️ 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".
| printf '%s\n' "[engraphis] refusing changed trusted config directory: $config_parent" >&2 | ||
| exit 1 | ||
| fi | ||
| if ! chown engraphis:engraphis "$config_parent"; then |
There was a problem hiding this comment.
Transfer ownership of every newly created config ancestor
When an allowed absolute config path has multiple missing directories under an app-owned ancestor (for example, /config/new/deep/config.env), the 077 umask makes every component created by mkdir -p root-owned and mode 0700, but this line transfers only deep; after gosu, engraphis cannot traverse new, so loading the required explicit config fails and the container exits. Fresh evidence in the current tree is that the new final-parent chown still leaves intermediate components created by the same mkdir -p untouched; transfer each newly created ancestor safely or reject paths requiring multiple new components.
AGENTS.md reference: AGENTS.md:L48-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07cf507a1d
ℹ️ 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".
| if [ ! -e "$ownership_marker" ]; then | ||
| if ! chown -R engraphis:engraphis /data; then |
There was a problem hiding this comment.
Recheck descendants before trusting the ownership marker
When /data is restored from an archive or snapshot that includes .volume-ownership but recreates files as root, this branch skips the recursive repair and only chowns /data, the state directory, and the marker. Consequently, a root-owned /data/engraphis.db or cache subtree remains inaccessible after gosu, preventing the restored service from starting; moving the marker onto /data fixes replacement volumes without a marker, but does not make the documented restored-volume case safe.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6debb317c
ℹ️ 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".
| printf '%s\n' "[engraphis] unable to create trusted config file: $config_file" >&2 | ||
| exit 1 | ||
| fi | ||
| if ! reject_linked_path "$config_file" || [ ! -f "$config_file" ]; then |
There was a problem hiding this comment.
Reject hard-linked trusted config files before mutating them
When a restored or externally provisioned config.env has multiple hard links, this regular-file check accepts it, so the root entrypoint applies chmod and later chown to the shared inode and therefore every alias. After privileges are dropped, read_private_text() rejects the same file because its link count is not one, causing startup to fail after unrelated aliases have already been modified; validate the link count before either privileged mutation.
Useful? React with 👍 / 👎.
Railway cold starts and restored volumes could leave the dashboard unable to persist settings or open its database. Startup now initializes trusted settings at
/data/.engraphis/config.env, validates ownership and path boundaries, and preserves the port and readiness contract across restarts./data, scan for restored descendants with incorrect ownership, and repair without following external symlinks. Correctly owned volumes avoid recursive ownership writes but still require a metadata scan. External state and config parents must be provisioned for the app UID; external trees are not recursively repaired.Validation on
c830f9e7e5100f595dc049838f2863435884259a: all 29 GitHub checks passed, including the Docker health smoke and offline Python matrix. Nineteen isolated root startup cases, eight POSIX regression cases, and separate actual UID 10001 startup/restart checks passed. The branch merges cleanly with current main and PR #225.The shared startup implementation and regressions are identical in PR #225. No production deployment is included.