fix: fall back to AdminPassword from PalWorldSettings.ini when ADMIN_PASSWORD is unset

[Cabecinha84]

What this fixes

Every container side REST API call authenticates with admin:${ADMIN_PASSWORD}
(helper_functions.sh REST_API()). That is the only source of the credential.

When DISABLE_GENERATE_SETTINGS=true, compile-settings.sh never runs, so the .ini
becomes the source of truth and nothing populates ADMIN_PASSWORD from it. There is no
.ini → env path in the image. For anyone who sets the admin password only in
PalWorldSettings.ini — which is exactly what the DISABLE_GENERATE_SETTINGS docs tell
users to do — ADMIN_PASSWORD stays empty and every REST call returns Unauthorized.

Two user visible consequences:

1. Auto reboot never runs. auto_reboot.sh gets a 401 on get_player_count (which
   returns 0 with rc 0, so the players-online guard passes silently), then
   shutdown_server refuses to shut down because save_server 401s — "Do not shutdown if
   not able to save". The cron job exits 1 a couple of seconds after it starts, with no
   error beyond supercronic's own line:

   level=info msg=starting job.command="bash /home/steam/server/auto_reboot.sh" job.schedule="0 5 * * *"
   [ERROR] error running command: exit status 1
   

2. No graceful shutdown ever saves. init.sh term_handler calls the same
   shutdown_server; when it fails it falls through to
   kill -SIGTERM "$(pidof PalServer-Linux-Shipping)" under a # Does not save comment. So
   docker stop, restarts and redeploys all discard everything since the last autosave.

Same root cause for rcon.yaml, which is written with an empty password.

The change

get_admin_password() returns ADMIN_PASSWORD when it is set, and otherwise reads
AdminPassword from PalWorldSettings.ini. REST_API() and the rcon.yaml heredoc use it.

• ADMIN_PASSWORD keeps precedence, so behaviour is unchanged for every setup that sets it.
• With DISABLE_GENERATE_SETTINGS=false the .ini is generated from ADMIN_PASSWORD, so the
  fallback is a no-op there too.
• Resolution happens at call time, so it also covers term_handler in init.sh — the parent
  process, whose environment nothing else can reach.
• Only the quoted form the server and compile-settings.sh write (AdminPassword="…") is
  matched; an unquoted hand edited value falls back to today's behaviour.
• start.sh logs a line when the fallback applies, so it is visible in the container log.

Deliberately out of scope: RESTAPIPort / RESTAPIEnabled have the same env vs .ini split
but are not part of this bug, so they are untouched.

Testing

• tests/test-admin-password.sh (new, wired into unit-test.yml) covers: value read from the
  file, special characters preserved, CRLF stripped, empty AdminPassword rejected, missing
  file rejected, ADMIN_PASSWORD precedence, .ini used when ADMIN_PASSWORD is empty, and an
  unset ADMIN_PASSWORD tolerated.
• ShellCheck clean with .shellcheckrc (external-sources=true) on every script in the repo.
• The failure was reproduced on a live server running this image with DISABLE_GENERATE_SETTINGS=true
  and the password set only in the .ini: curl -u "admin:$ADMIN_PASSWORD" …/v1/api/players returns
  401 while the same call with the .ini password returns 200, and the nightly auto_reboot.sh job
  exits 1 every night. With this patch applied, REST_API() builds the call with the .ini password
  (checked with a stubbed curl), and with ADMIN_PASSWORD set it is unchanged.
• Pure bash and POSIX tr, no GNU only flags, nothing architecture specific.

AI disclosure

Per AI_GUIDELINES.md: this patch was written with the help of an AI assistant. The failure was
diagnosed on a live production server first, the diff was reviewed by hand, and the tests and
ShellCheck run were executed locally rather than assumed.

[Copilot]

Pull request overview

Adds an INI-based admin password fallback so internal REST API and RCON operations authenticate when settings generation is disabled.

Changes:

• Resolves AdminPassword from the environment or INI file.
• Applies the resolved password to REST API and RCON clients.
• Adds documentation and unit tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
scripts/helper_functions.sh	Implements password resolution and REST API integration.
scripts/start.sh	Logs fallback usage and configures RCON credentials.
tests/test-admin-password.sh	Tests password resolution behavior.
.github/workflows/unit-test.yml	Runs the new tests.
docusaurus/docs/getting-started/configuration/game-settings.md	Documents fallback behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Cabecinha84]

Confirmed, and fixed in 52e2184.

I reproduced it against a YAML parser before changing anything: in a double quoted scalar pa\ssw0rd fails to parse, tab\there is silently rewritten with a real tab, and a password holding " breaks the mapping. Worth noting the same hole already existed on the ${ADMIN_PASSWORD} line this PR replaces, so this is not new behaviour, but the fallback is a good moment to close it.

The password is now written as a YAML single quoted scalar, which processes no escape sequences at all and only needs a literal quote doubled:

password: 'pa\ss"w0rd''x'

yaml_single_quoted() in helper_functions.sh does the doubling, start.sh renders the scalar before the heredoc, and tests/test-admin-password.sh covers backslashes, double quotes and single quotes. Round tripping the generated rcon.yaml through a parser returns the password unchanged.

[Cabecinha84]

Pushed 52e2184 after a self review of the code this PR adds. Copilot's rcon.yaml point was real, and going back over get_admin_password_from_settings() line by line turned up three more cases it got wrong. All four are fixed, each has a test, and every test fails against the previous commit.

Input	Before	After
AdminPassword="secret"	secret	secret
AdminPassword = "secret"	no match	secret
;OptionSettings=(AdminPassword="OLD") above the live line	OLD	live value
AdminPassword="" above the real one	no match, no fallback	real value
password written to rcon.yaml	double quoted scalar, \ breaks the file	single quoted scalar, literal

The last two are the ones that bothered me most. A bash regex only ever matches the first occurrence, so [[ "${settings}" =~ $pattern ]] && [ -n "${BASH_REMATCH[1]}" ] gave up on an empty value instead of looking further, which meant a file carrying a stale empty AdminPassword fell straight back into the bug this PR is meant to fix, silently. And a hand edited .ini is exactly the audience for this fallback, so tolerating spaces around the assignment and ignoring commented out lines seemed worth the ten lines. The function now reads the file line by line, skips comments and keeps the last non empty value.

Scope is unchanged: only the function this PR introduces and the one line it already touched. ShellCheck 0.10 is clean on every script, and ./tests/test-admin-password.sh and ./tests/test-negative-delta-recovery.sh both pass locally.

@thijsvanloef two things when you get a chance:

1. The Unit-test workflow has never run on this PR, so the new test has not been exercised in CI. It needs your approval to run.

2. One design question I deliberately did not decide on my own. Right now ADMIN_PASSWORD wins whenever it is set, which keeps existing setups untouched. But with DISABLE_GENERATE_SETTINGS=true the container already logs Env vars will not be applied due to DISABLE_GENERATE_SETTINGS being set to TRUE!, and the .ini is by definition what the server authenticates against. So anyone who has ADMIN_PASSWORD set to something different from the .ini value, which is easy to end up with after copying a compose file and enabling DISABLE_GENERATE_SETTINGS later, still gets Unauthorized on every container side call, and this PR does not help them at all, because a non empty env var blocks the fallback.

   Letting the .ini win in that mode would match what the container already tells the user, and would be a no op for anyone who has both set to the same value. It is a behaviour change though, so I left the safe precedence in place. Happy to push it if you prefer it.