admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

case 4690 - erase database: check CREATEDB before dropping, so a failed create can't leave no database

Browse Source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-22 20:15:13 -07:00
parent 78f5863947
commit e8e7a2ec40
2 changed files with 15 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
devdocs/modernization/NET10-PORT.md
View File

@@ -116,7 +116,7 @@ in this branch or a follow-up release. UPDATE: John has decided these all go in
- [x] **Case 4676: data-list SQL injection — priority.** Confirmed: `DataListSqlFilterCriteriaBuilder` appended the raw filter value for Roles and for every numeric type (Enum, MemorySize, Decimal, Currency, InternalId, Integer). Text and tag values escape `'`; TimeSpan goes through `int.Parse`. Any authenticated user who can open a data list could reach this. **Sockeye's fix (`3427cd8`) is incomplete:** it checks only Decimal/Currency and Integer, and leaves InternalId, Enum, MemorySize and Roles open (still open in Sockeye 8.1.0). AyaNova's fix parses every numeric type as an invariant `decimal` (Decimal, Currency) or `long` (the rest), and Roles as a `long`, then appends the re-formatted number. An invalid value throws `ArgumentException`, which the API returns as an error before any SQL runs
- [x] **`DbUtil.AdminConnectionString`.** Fixed with the builder below (Sockeye 8.1.0 still has the `Replace`). Confirmed: `DbUtil.cs:99` is `_dbConnectionString.Replace(_dbName, "postgres")`, used by the boot check (`:192`), create (`:286`) and drop/recreate (`:334`). Any customer whose role name, password or host contains the database name as text gets a mangled admin connection, and the boot check authenticates as the wrong user. Fix: `new NpgsqlConnectionStringBuilder(cs) { Database = "postgres" }.ConnectionString`
- [x] **Backup `pg_dump` call.** Fixed (Sockeye 8.1.0 still has the old call): new `RunProgram.RunNoShell` (no shell, `ArgumentList`, per-process environment, both streams read together, timeout kills the process tree, non-zero exit code returned as an error); backup passes `--host/--port/--username/--dbname/--format=custom/--file/--no-password` with the password in `PGPASSWORD`. The old `RunProgram.Run` now has no callers (see Deferred). Confirmed: `CoreJobBackup.cs:68` builds `postgresql://user:password@host:port/db` with the raw password, and `:94` adds `-Fc > file`; `RunProgram` runs it through `cmd.exe /C` (`RunProgram.cs:55`) or `/bin/sh -c` (`:82-89`). A password with spaces or `@ : / ? # %` breaks backups. Also, on Linux stderr is not redirected (`RunProgram.cs:86-92`), so a failed pg_dump returns empty output and is **reported as a successful backup** (`CoreJobBackup.cs:97`). Fix: password in `PGPASSWORD` in the process environment, connection parameters as separate arguments through `ProcessStartInfo.ArgumentList` (no shell), output with `-f`, stderr and exit code checked
- [ ] **Drop-and-recreate database.** Confirmed: `DbUtil.cs:342-345` drops, then creates; triggered by `AYANOVA_PERMANENTLY_ERASE_DATABASE` (`Startup.cs:567-570`). Without `CREATEDB` it drops the database and then fails to create it. Test the permission before dropping, or change the order
- [x] **Drop-and-recreate database.** Fixed: before the `DROP`, `DropAndRecreateDbAsync` checks `rolcreatedb OR rolsuper` for the current user; without it, boot stops with an E1000 message naming the user and the setting, and the database is untouched. Confirmed: `DbUtil.cs:342-345` drops, then creates; triggered by `AYANOVA_PERMANENTLY_ERASE_DATABASE` (`Startup.cs:567-570`). Without `CREATEDB` it drops the database and then fails to create it. Test the permission before dropping, or change the order
- [ ] **Report browser download location.** Confirmed: `new BrowserFetcher()` with the default path beside the app (`ReportBiz.cs:800`). Point it under the data directory in code. Beside the app it is read-only under a hardened systemd unit (`ProtectSystem=strict`), and a redeploy that replaces the app folder deletes it
- [ ] **Report failures: return a specific error.** The Sockeye crash is n/a: rendering runs as a job, and `JobsBiz.ProcessJobAsync` catches every exception, logs it through NLog and marks the job Failed (`JobsBiz.cs:390-439`), so the process survives. What remains: a browser download or launch failure (`ReportBiz.cs:800`, `:838`, outside the render `try`) reaches the client only as a generic job failure. Catch it in `DoRenderJob` and return a render error that says what failed
- [ ] **Data-list date filters, small fixes.** Confirmed: `>=` (`:790-793`) and `<` (`:795-798`) use `MaxSeconds` and should use `ZeroSeconds`; date `NotEqual` (`:806-814`) uses `MaxSeconds` on both sides and should exclude the whole minute (`< ZeroSeconds OR > MaxSeconds`). `DataListField` is dereferenced at `:24` and `:27` before the DEBUG-only null check at `:33`, so **release builds** throw a NullReferenceException on an unknown column, not only DEBUG builds
@@ -208,3 +208,4 @@ Found during the port, out of scope. One line each.
| 2026-10-22 | CA2022: `FileUtil.GetLinkerTimestampUtc` reads with `ReadAtLeast` | CA2022 gone (only AV0015/AV0016 left). Licence gate 730/730 incl. build-timestamp and `MExBB` checks; per-row outcomes identical to the first .NET 10 run (389 ok / 337 rejected for this build). Server boots and accepts the (trial) licence |
| 2026-10-22 | Bogus 35.6.5, jose-jwt 5.3.0, QRCoder 1.8.1, Newtonsoft.Json 13.0.4 direct; `appsettings.json` levels; ASP0019 n/a | Builds (only AV0015/AV0016). All packages current, none vulnerable. Login JWT still `HS256` and accepted on every authenticated call; `GET auth/totp` returns a valid 183×183 QR PNG and leaves 2FA disabled. Integration suite = baseline. Bogus (evaluation seeder) compiled only: running the seeder replaces the test data |
| 2026-10-22 | Backup `pg_dump` without a shell; password in `PGPASSWORD` | On-demand backups through the API into a scratch folder with a space in its name, checked with `pg_restore --list`. Old code as `postgres`: 803 entries. New code as `postgres`: 803 entries. Failing stand-in `pg_dump` (exit code 2): reported as `BACKUP ERROR`, no file, no password in the log. Temporary superuser role `"AyaNova"` (named like the database) with password `p @:/?#% x`: old code failed (`too many command-line arguments (first is "@:/?#%")`), left a 0-byte backup file and echoed part of the password into the log; new code 803 entries, no errors. Role dropped afterwards. Integration suite = baseline. Linux not tested here: the stderr gap there is closed by the same code (no shell, exit code checked) |
| 2026-10-22 | Drop-and-recreate: CREATEDB check before the drop | Throwaway non-superuser role `ayatest_nocreatedb` owning throwaway database `ayatest_erase`, server data path in scratch (the erase also clears the attachments folder). Without CREATEDB: boot stops with the new E1000 message, database still there. Old sequence by hand as the same role: `DROP DATABASE` succeeds, `CREATE DATABASE` fails "permission denied", database gone. With CREATEDB: erased and recreated (oid 145595 → 145596), schema 20 built, boot complete. Database and role dropped afterwards. Integration suite not rerun: the change is only reached with the erase setting |

13
server/AyaNova/util/DbUtil.cs
View File

@@ -336,6 +336,19 @@ namespace AyaNova.Util
{
await conn.OpenAsync();
//Without CREATEDB the DROP below would succeed and the CREATE after it fail, leaving no database at all
//so check first and stop with the database untouched
using (var cmdCheck = new Npgsql.NpgsqlCommand("SELECT rolcreatedb OR rolsuper FROM pg_roles WHERE rolname = current_user;", conn))
{
var canCreate = await cmdCheck.ExecuteScalarAsync();
if (!(canCreate is bool b && b))
{
var err = $"E1000 - AYANOVA_PERMANENTLY_ERASE_DATABASE is set but database user \"{_dbUserName}\" does not have permission to create databases (CREATEDB). The database \"{_dbName}\" has NOT been erased. Grant CREATEDB to this user or erase the database another way, then remove the setting";
_log.LogCritical(err);
throw new ApplicationException(err);
}
}
// Create the database desired
using (var cmd = new Npgsql.NpgsqlCommand())
{