diff --git a/deploy/README.md b/deploy/README.md index 3d1d5f84..751e18d8 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -3,3 +3,7 @@ The deployment script and Gitea workflow live in this directory and `.gitea/workflows/`. The current deployment, health/readiness, rollback, and secret-handling guidance is in the repository [infrastructure guide](../docs/infrastructure/README.md), especially [deployment](../docs/infrastructure/deployment.md) and [disaster recovery](../docs/infrastructure/disaster-recovery.md). Keep the server `.env` outside the repository. Do not run production deployment, restore, migration, or credential rotation as part of local documentation work. + +MariaDB backup connection parsing uses `scripts/parse-backup-connection.py` (Python 3, already required by deployment preflight). Keys are case-insensitive and support the documented host/user/password/database aliases. Single/double-quoted values may contain semicolons and doubled quote characters. The dump contract requires one TCP host, a database and user, with port 1-65535 (default 3306); an empty password is retained. Duplicate aliases, control characters, host lists/socket paths and database path separators are rejected before dumping. This deliberately bounded parser is not the full MySqlConnector configuration engine. Additional connection options do not change existing dump-client TLS behavior; that qualification remains separate. + +Run `python scripts/test-deploy-backup.py` for the offline synthetic dump-boundary regression. It extracts only the backup function and never executes the deployment entry point, contacts a database or reads the production environment. diff --git a/deploy/deploy.sh b/deploy/deploy.sh index 49e142a7..8341110d 100644 --- a/deploy/deploy.sh +++ b/deploy/deploy.sh @@ -326,19 +326,19 @@ backup_database() { return 1 fi - # Parse the ADO.NET connection string. Keys are case-insensitive in .NET, so match that. - local db_host db_port db_name db_user db_pass - db_host="$(sed -n 's/.*[Ss]erver=\([^;]*\).*/\1/p' <<<"$cs")" - db_port="$(sed -n 's/.*[Pp]ort=\([^;]*\).*/\1/p' <<<"$cs")" - db_name="$(sed -n 's/.*[Dd]atabase=\([^;]*\).*/\1/p' <<<"$cs")" - db_user="$(sed -n 's/.*[Uu]ser[ ]*[Ii]*[Dd]*=\([^;]*\).*/\1/p' <<<"$cs")" - db_pass="$(sed -n 's/.*[Pp]assword=\([^;]*\).*/\1/p' <<<"$cs")" - db_port="${db_port:-3306}" - - if [ -z "$db_host" ] || [ -z "$db_name" ] || [ -z "$db_user" ]; then - echo "Could not parse host/database/user from JOBTRACKER_CONNECTION_STRING. Aborting deploy." + # Keep secrets off argv; parse aliases/quoted separators without evaluating shell text. + local db_host db_port db_name db_user db_pass parsed_connection + local -a connection_fields=() + if ! parsed_connection="$(printf '%s' "$cs" | python3 scripts/parse-backup-connection.py)"; then + echo "Could not parse JOBTRACKER_CONNECTION_STRING for backup. Aborting deploy." return 1 fi + mapfile -t connection_fields <<< "$parsed_connection" + db_host="${connection_fields[0]}" + db_port="${connection_fields[1]}" + db_name="${connection_fields[2]}" + db_user="${connection_fields[3]}" + db_pass="${connection_fields[4]}" local target="$BACKUP_DIR/jobtracker-${db_name}-${BACKUP_STAMP}.sql.gz" echo "Backing up MariaDB database '${db_name}' on ${db_host}:${db_port} to ${target}" diff --git a/docs/release-readiness/RR-11_IMPLEMENTATION.md b/docs/release-readiness/RR-11_IMPLEMENTATION.md new file mode 100644 index 00000000..dce1dacd --- /dev/null +++ b/docs/release-readiness/RR-11_IMPLEMENTATION.md @@ -0,0 +1,19 @@ +# RR-11 bounded remediation + +Authorised by the owner's Continuous Roadmap Execution instruction explicitly prioritising this fix. Branch `codex/release-backup-parser`, based on published discovery `653f8d04d469dcd52c883d4b98bea76b8adacada`. Separate from CLEAN-20, full release qualification and infrastructure operations. + +Replaced five inconsistent sed expressions with one stdlib Python parser and a shell-safe field transfer. Case-insensitive aliases, quoted separators/doubled quotes, Unicode and empty passwords survive to the dump command. Invalid ports, incomplete/ambiguous/control-bearing values and unsupported multi-host/socket/path forms fail before dumping. No eval; connection input travels via stdin, not argv; errors never include input. No dependency added. Existing dump/TLS/retention/rollback behavior is otherwise unchanged. + +## Evidence + +- Regression first: four test methods ran against the original backup function; eight subcase failures reproduced alias/case rejection and quoted-password truncation. Canonical/negative controls executed. No real dump or deployment. +- Final targeted suite: **5/5 methods PASSED**, including 7 host spellings, quoted semicolon/equal/doubled quote/shell characters, Unicode/single quotes, empty password, and six invalid/ambiguous input cases. Tests run the actual backup function and mock only external dump/Docker boundaries. +- Full offline script unittest set: **31/31 PASSED**, including targeted cases, ingress Compose rendering with synthetic env, workflow/runner guards, retry guard, supply-chain fixtures and offline model-evaluation helpers. No model invocation. Printed exit17/exit1 are expected mocked runner negative cases. Do not add targeted five to the overlapping total. +- Ordinary unittest discovery found zero tests because repository filenames contain hyphens; recorded as NOT A PASS. Explicit importlib loading of every `scripts/test-*.py` unittest module (excluding standalone Docker topology runner) executed the 31 tests. Docker topology qualification NOT RUN. +- Git Bash `bash -n deploy/deploy.sh`: PASSED. +- Graphify AST update completed locally: 18,504 nodes / 45,920 edges; missing optional PDF/SQL extractors reported. Newly generated untracked graph output retained locally, not a runtime verification or introduced repository artifact. +- Product/backend/frontend/full database/browser suites NOT RUN: this package changes only deploy parsing and its offline tests. Real dump/restore, Linux-host operational behavior, hosted CI and production validation remain NOT TESTED. + +The first Unicode test attempt failed in the Windows test harness encoding before reaching product code; the harness was corrected to UTF-8, then passed. Parser stdin/stdout are explicitly UTF-8. No application user data or production credentials used. + +Rollback: revert this bounded implementation commit to restore the prior parser (and its known limitation). No database/schema change needs reversal. RR-11 is FIXED AND OFFLINE-TESTED on this development branch; RR-12/13 and production remain open. No main merge/deploy authorised. diff --git a/scripts/parse-backup-connection.py b/scripts/parse-backup-connection.py new file mode 100644 index 00000000..9aafba93 --- /dev/null +++ b/scripts/parse-backup-connection.py @@ -0,0 +1,68 @@ +"""Read a deployment connection string from stdin; emit five fields plus a sentinel. + +This is a single-host dump contract, not a replacement for MySqlConnector. Reject +ambiguous aliases/control characters instead of choosing a different backup target. +Never include input values in errors or pass the connection string as an argument. +""" +import re +import sys + + +ALIASES = { + **dict.fromkeys(['server', 'host', 'data source', 'datasource', 'address', 'addr', 'network address'], 'host'), + **dict.fromkeys(['user id', 'userid', 'username', 'uid', 'user name', 'user'], 'user'), + **dict.fromkeys(['password', 'pwd'], 'password'), + **dict.fromkeys(['database', 'initial catalog'], 'database'), + 'port': 'port', +} +PAIR = re.compile(r'''\s*([^=;]+?)\s*=\s*(?:"((?:""|[^"])*)"|'((?:''|[^'])*)'|([^;]*))\s*(?:;|$)''') + + +def parse(connection): + if any(ord(c) < 32 or ord(c) == 127 for c in connection): + raise ValueError('Control characters are unsupported in backup connection settings.') + fields = {} + position = 0 + while position < len(connection): + if connection[position] == ';' or connection[position].isspace(): + position += 1 + continue + match = PAIR.match(connection, position) + if not match: + raise ValueError('Malformed backup connection settings.') + key, double, single, plain = match.groups() + if double is not None: + value = double.replace('""', '"') + elif single is not None: + value = single.replace("''", "'") + else: + value = plain.strip() + if value.startswith(('"', "'")): + raise ValueError('Malformed quoted backup connection value.') + canonical = ALIASES.get(key.strip().lower()) + if canonical: + if canonical in fields: + raise ValueError('Duplicate backup connection setting or alias.') + fields[canonical] = value + position = match.end() + if any(not fields.get(key, '').strip() for key in ['host', 'database', 'user']): + raise ValueError('Backup connection requires host, database and user.') + port = fields.get('port', '3306') + if not re.fullmatch(r'[0-9]+', port) or not 1 <= int(port) <= 65535: + raise ValueError('Backup connection port must be between 1 and 65535.') + if any(c in fields['host'] for c in ',/\\'): + raise ValueError('Backup requires one TCP host, not a host list or socket path.') + if any(c in fields['database'] for c in '/\\'): + raise ValueError('Database name cannot contain backup path separators.') + return [fields['host'], str(int(port)), fields['database'], fields['user'], fields.get('password', '')] + + +if __name__ == '__main__': + try: + values = parse(sys.stdin.buffer.read().decode('utf-8')) + except (ValueError, UnicodeError): + print('Invalid or unsupported backup connection settings.', file=sys.stderr) + sys.exit(1) + sys.stdout.reconfigure(encoding='utf-8', newline='\n') + # Sentinel keeps an empty final password intact through shell command substitution. + sys.stdout.write('\n'.join([*values, 'END']) + '\n') diff --git a/scripts/test-deploy-backup.py b/scripts/test-deploy-backup.py new file mode 100644 index 00000000..5053be50 --- /dev/null +++ b/scripts/test-deploy-backup.py @@ -0,0 +1,89 @@ +"""Exercise the real backup function with a synthetic dump command; never deploy.""" +import os +from pathlib import Path +import shlex +import shutil +import subprocess +import sys +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +BASH = 'C:/Program Files/Git/bin/bash.exe' if os.name == 'nt' else shutil.which('bash') + + +class BackupConnectionTests(unittest.TestCase): + def backup(self, connection): + source = (ROOT / 'deploy/deploy.sh').read_text(encoding='utf-8') + start = source.index('backup_database() {') + function = source[start:source.index('\n}\n', start) + 3] + with tempfile.TemporaryDirectory() as temporary: + folder = Path(temporary) + script = '\n'.join([ + 'set -euo pipefail', + 'BACKUP_DIR=' + shlex.quote(folder.as_posix()), + 'BACKUP_STAMP=synthetic', + 'DATABASE_PROVIDER=mariadb', + 'JOBTRACKER_CONNECTION_STRING=' + shlex.quote(connection), + 'python3() { ' + shlex.quote(Path(sys.executable).as_posix()) + ' "$@"; }', + 'docker() { echo "Unexpected Docker call" >&2; return 99; }', + 'mariadb-dump() { printf "%s\\0" "$@" > "$BACKUP_DIR/args"; ' + 'printf "%s" "$MYSQL_PWD" > "$BACKUP_DIR/password"; ' + 'printf "CREATE TABLE synthetic (id int);\\n-- Dump completed\\n"; }', + 'verify_sql_backup() { gzip -t "$1"; }', + function, + 'backup_database', + ]) + result = subprocess.run([BASH, '--noprofile', '--norc'], input=script, + text=True, encoding='utf-8', capture_output=True, cwd=ROOT, + env={**os.environ, 'DEPLOY_SKIP_DB_BACKUP': 'false'}) + arguments = (folder / 'args').read_bytes().split(b'\0')[:-1] if (folder / 'args').exists() else None + password = (folder / 'password').read_text(encoding='utf-8') if (folder / 'password').exists() else None + return result, arguments, password + + def test_supported_host_alias_and_case_reach_dump_unchanged(self): + for host in ['Server', 'SERVER', 'Host', 'Data Source', 'Address', 'Addr', 'Network Address']: + with self.subTest(host=host): + result, args, password = self.backup(f'{host}=127.0.0.1;DATABASE=synthetic;UID=reader;PWD=dummy-secret') + self.assertEqual(0, result.returncode, result.stderr + result.stdout) + self.assertEqual([b'--host=127.0.0.1', b'--port=3306', b'--user=reader', + b'--single-transaction', b'--routines', b'--events', b'--quick', b'synthetic'], args) + self.assertEqual('dummy-secret', password) + self.assertNotIn('dummy-secret', result.stdout + result.stderr) + + def test_quoted_separators_and_shell_characters_remain_data(self): + result, args, password = self.backup('Server=127.0.0.1;Port=3307;Database=synthetic;User ID=reader;Password="semi;eq=quote""$(false)"') + self.assertEqual(0, result.returncode, result.stderr + result.stdout) + self.assertIn(b'--port=3307', args) + self.assertEqual('semi;eq=quote"$(false)', password) + + def test_empty_password_is_preserved(self): + result, args, password = self.backup('Server=127.0.0.1;Database=synthetic;User ID=reader;Password=') + self.assertEqual(0, result.returncode, result.stderr + result.stdout) + self.assertIsNotNone(args) + self.assertEqual('', password) + + def test_single_quoted_unicode_password_and_database_alias_are_preserved(self): + result, args, password = self.backup("Host=127.0.0.1;Initial Catalog=synthetic;Username=reader;Password='søknad你好;it''s';SslMode=VerifyFull") + self.assertEqual(0, result.returncode, result.stderr + result.stdout) + self.assertEqual(b'synthetic', args[-1]) + self.assertEqual("søknad你好;it's", password) + + def test_invalid_or_ambiguous_input_never_invokes_dump_or_exposes_input(self): + for connection in [ + 'Host=127.0.0.1;Database=synthetic;Uid=reader;Port=0;Pwd=dummy-secret', + 'Host=127.0.0.1;Database=synthetic;Uid=reader;Port=65536;Pwd=dummy-secret', + 'Host=127.0.0.1;Server=other;Database=synthetic;Uid=reader;Pwd=dummy-secret', + 'Host=127.0.0.1;Database=synthetic;Uid=reader;Pwd="dummy-secret', + 'Host=127.0.0.1;Database=synthetic;Uid=reader;Pwd=dummy-secret\nline', + 'Host=127.0.0.1;Uid=reader;Pwd=dummy-secret', + ]: + with self.subTest(connection=connection.replace('dummy-secret', 'REDACTED')): + result, args, _ = self.backup(connection) + self.assertNotEqual(0, result.returncode) + self.assertIsNone(args) + self.assertNotIn('dummy-secret', result.stdout + result.stderr) + + +if __name__ == '__main__': + unittest.main()