fix(deploy): parse backup connection aliases and quoted values safely
This commit is contained in:
@@ -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).
|
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.
|
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.
|
||||||
|
|||||||
+11
-11
@@ -326,19 +326,19 @@ backup_database() {
|
|||||||
return 1
|
return 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Parse the ADO.NET connection string. Keys are case-insensitive in .NET, so match that.
|
# Keep secrets off argv; parse aliases/quoted separators without evaluating shell text.
|
||||||
local db_host db_port db_name db_user db_pass
|
local db_host db_port db_name db_user db_pass parsed_connection
|
||||||
db_host="$(sed -n 's/.*[Ss]erver=\([^;]*\).*/\1/p' <<<"$cs")"
|
local -a connection_fields=()
|
||||||
db_port="$(sed -n 's/.*[Pp]ort=\([^;]*\).*/\1/p' <<<"$cs")"
|
if ! parsed_connection="$(printf '%s' "$cs" | python3 scripts/parse-backup-connection.py)"; then
|
||||||
db_name="$(sed -n 's/.*[Dd]atabase=\([^;]*\).*/\1/p' <<<"$cs")"
|
echo "Could not parse JOBTRACKER_CONNECTION_STRING for backup. Aborting deploy."
|
||||||
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."
|
|
||||||
return 1
|
return 1
|
||||||
fi
|
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"
|
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}"
|
echo "Backing up MariaDB database '${db_name}' on ${db_host}:${db_port} to ${target}"
|
||||||
|
|||||||
@@ -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.
|
||||||
@@ -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')
|
||||||
@@ -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()
|
||||||
Reference in New Issue
Block a user