Release V-01.05 hardening and package docs#5
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0a019dccd
ℹ️ 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".
| POSTGRES_PASSWORD: ${ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD:?Set ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD in a local .env file or environment variable} | ||
| ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD: ${ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD:?Set ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD in a local .env file or environment variable} | ||
| ATLAS_BALANCE_POSTGRES_APP_PASSWORD: ${ATLAS_BALANCE_POSTGRES_APP_PASSWORD:?Set ATLAS_BALANCE_POSTGRES_APP_PASSWORD in a local .env file or environment variable} |
There was a problem hiding this comment.
This change makes docker compose up fail hard unless ATLAS_BALANCE_POSTGRES_OWNER_PASSWORD and ATLAS_BALANCE_POSTGRES_APP_PASSWORD are both defined, because all three password entries now use the :? required-variable form. Environments that were previously valid with the single DB password variable (the current repo example still uses that shape) will now stop booting immediately after upgrading, so local/dev startup and automation break until every caller is manually migrated.
Useful? React with 👍 / 👎.
| if (string.IsNullOrWhiteSpace(healthUrl)) | ||
| { | ||
| healthUrl = "https://localhost/api/health"; | ||
| } |
There was a problem hiding this comment.
When WatchdogSettings:ApiHealthUrl is not set, the fallback probes https://localhost/api/health, which implicitly assumes port 443. With health checks now enabled by default in this release flow, installations running the API on another HTTPS port will repeatedly mark updates as failed and trigger rollback even when binaries are healthy, because the probe never reaches the service.
Useful? React with 👍 / 👎.
|
Version 1.5 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 838a76b5a1
ℹ️ 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".
| var dbUser = _configuration["WatchdogSettings:DbUser"] ?? "app_user"; | ||
|
|
||
| var result = await RunProcessAsync( | ||
| executable, | ||
| ["-h", dbHost, "-p", dbPort.ToString(), "-U", dbUser, "-F", "c", "-b", "-f", backupPath, dbName], | ||
| dbPassword, |
There was a problem hiding this comment.
The new pre-update backup path now shells out to pg_dump using WatchdogSettings:DbUser/DbPassword, which is the runtime app role in hardened installs. In this same release, startup provisioning revokes access to security metadata tables from that role, so pg_dump can fail with permission errors and abort every update when RequireDatabaseBackupBeforeUpdate is enabled (it is true in the new templates). This blocks updates in the split owner/app-role setup introduced by this commit unless the dump is executed with owner credentials (or equivalent privileges).
Useful? React with 👍 / 👎.
| var secret = setupRequired | ||
| ? TotpService.GenerateSecret() | ||
| : _secretProtector.UnprotectFromStorage(usuario.MfaSecret) ?? string.Empty; | ||
|
|
There was a problem hiding this comment.
When an MFA-enabled user logs in, CreateMfaChallenge directly decrypts usuario.MfaSecret; if Data Protection keys changed or are unavailable, this throws and bubbles as a 500 instead of a recoverable auth flow. Because this code already has a fallback path for missing/blank secrets, this uncaught decrypt failure effectively locks out affected users rather than prompting re-enrollment. Catching decrypt failures here and switching to setup-required would avoid production lockouts after key-store issues or restores.
Useful? React with 👍 / 👎.
1 participant
No description provided.