Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions docs/doctoring/bootstrap-dsn-precedence.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

For the required Postgres DSN, an explicitly empty or whitespace-only value must not be replaced by `PG_LLM_BATCH_DSN` or passed down to libpq defaults. Non-string explicit values must also fail at the package boundary rather than reaching unrelated lower-layer behavior. For the optional Fernet bootstrap key, an explicit empty string is a deliberate statement that no key was supplied for this invocation and must not inherit an ambient key.

A separate CLI confidentiality boundary applies before bootstrap resolution. PostgreSQL connection information can carry passwords, password-file locations, TLS private-key material, TLS key passwords, and OAuth client secrets. Accepting those values through `--dsn` copies credential material or credential-bearing locations into process invocation state, where operating-system process inspection and shell history can expose them. The CLI therefore needs to retain explicit database targeting without making credential-bearing conninfo a normal argv transport.

## Contract

`resolve_dsn()` distinguishes source absence, source type, and source value:
Expand All @@ -23,22 +25,44 @@ For the required Postgres DSN, an explicitly empty or whitespace-only value must
- an explicit empty string remains the empty string and does not inherit `PG_LLM_BATCH_SECRET_KEY`; and
- when neither source provides a key, the result remains `None`.

This boundary does not make secret persistence, serialization, or transport safer by itself. It only prevents ambient bootstrap state from silently replacing explicitly supplied caller intent.
The standalone CLI adds a narrower transport rule for explicit `--dsn` values:

- parse the supplied value with Psycopg/libpq-compatible `conninfo_to_dict()` rather than ad-hoc URI or keyword matching;
- permit credential-free selectors such as password-free PostgreSQL URIs, keyword conninfo, and `service=` selectors;
- reject conninfo that explicitly contains `password`, `passfile`, `sslkey`, `sslpassword`, or `oauth_client_secret` before bootstrap resolution or database connection work;
- reject malformed conninfo with a fixed parser diagnostic that does not reproduce the rejected argv value; and
- preserve the exact accepted selector string so downstream source-precedence and libpq semantics remain unchanged.

The CLI restriction does not prohibit standard libpq authentication. Operators may keep password/private-key material outside argv using reviewed libpq mechanisms such as the default password file, `PGPASSFILE`, a connection service file, default or environment-selected TLS key material, or deployment-owned secret injection. `PG_LLM_BATCH_DSN` remains a bootstrap transport and is not claimed to be a universal secrets manager; deployments should select an appropriate secret mechanism for their threat model.

This boundary does not make secret persistence, serialization, transport, TLS, or server identity safe by itself. It prevents two specific authority/confidentiality failures: ambient bootstrap state silently replacing explicit caller intent, and credential-bearing explicit CLI conninfo becoming process-argument data.

## Verification

`tests/test_bootstrap_source_precedence.py` proves the replacement behavior against the public bootstrap helpers. The regressions populate ambient environment values while passing explicit invalid values so a rejected caller value cannot be confused with ordinary omitted-input fallback. They also prove that an omitted whitespace-only DSN is rejected, a valid explicit DSN retains exact text, and an explicit empty secret key remains explicit.

The fail-first replacement head demonstrated that protected-main truthiness selected ambient values or admitted the wrong type before the production repair. The production repair then moved fallback behind an explicit `None` check and added exact-string and required-nonblank validation. Final acceptance still requires the repository's complete exact-head CI, security, coverage, package, provenance, and review policy on the unchanged final source.
`tests/test_cli_dsn_argv_security.py` defines the CLI transport contract. It requires password-bearing PostgreSQL URIs, keyword `password=`, and explicit `passfile=` values to fail without reflecting a unique secret sentinel; it separately requires malformed conninfo to fail without reflection and confirms that credential-free URI, keyword, and service selectors retain exact text.

The fail-first bootstrap replacement head demonstrated that protected-main truthiness selected ambient values or admitted the wrong type before the production repair. The CLI fail-first branch independently demonstrated that protected main accepted credential-bearing `--dsn` values unchanged. The CLI production repair uses libpq-compatible parsing only to classify whether argv contains prohibited credential parameters; it does not rewrite accepted connection information or change bootstrap precedence. Final acceptance still requires the repository's complete exact-head Python 3.10/3.12/3.14, 100% owned production statement/branch coverage, public docstrings, package, security, SAST, required-workflow, review-thread, and live ruleset evidence on one unchanged final source.

## Compatibility and rollback

Constructor and CLI call shapes remain unchanged. Callers that intentionally depended on explicit empty/non-string values falling through to environment state must now omit the argument to request environment fallback. Valid explicit DSNs and keys retain their original string values.
Bootstrap helper call shapes remain unchanged. Callers that intentionally depended on explicit empty/non-string values falling through to environment state must now omit the argument to request environment fallback. Valid explicit DSNs and keys retain their original string values.

Rollback is an ordinary Git revert of this bounded bootstrap change. Rolling back reintroduces ambiguous authority selection and should occur only with a documented compatibility requirement and a safer replacement contract.
The CLI keeps `--dsn` for explicit database selection but no longer accepts credentials or credential-file/private-key parameters in that process argument. Existing automation that embeds such material in `--dsn` must move authentication data to a standard libpq mechanism outside argv while preserving its database selector. This is an intentional confidentiality hardening, not silent credential removal.

Rollback is an ordinary Git revert of the bounded change. Rolling back the bootstrap rule reintroduces ambiguous authority selection; rolling back the CLI rule reintroduces credential-bearing process arguments. Either rollback should occur only with a documented compatibility requirement and a safer replacement contract.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## References

MITRE. (2026). *CWE-214: Invocation of process using visible sensitive information* (CWE Version 4.20). https://cwe.mitre.org/data/definitions/214.html

PostgreSQL Global Development Group. (2026). *PostgreSQL 18 documentation: Database connection control functions*. https://www.postgresql.org/docs/18/libpq-connect.html

PostgreSQL Global Development Group. (2026). *PostgreSQL 18 documentation: The password file*. https://www.postgresql.org/docs/18/libpq-pgpass.html

The Psycopg Team. (2026). *Psycopg 3 documentation: `conninfo` — manipulate connection strings*. https://www.psycopg.org/psycopg3/docs/api/conninfo.html

Python Software Foundation. (2026). *argparse — Parser for command-line options, arguments and subcommands*. Python 3.14 documentation. https://docs.python.org/3.14/library/argparse.html

Python Software Foundation. (2026). *os — Miscellaneous operating system interfaces*. Python 3.14 documentation. https://docs.python.org/3.14/library/os.html
42 changes: 38 additions & 4 deletions pg_llm_batch/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
serve-healthz serve GET /healthz

The DSN is resolved from --dsn or the PG_LLM_BATCH_DSN bootstrap env var only.
All other config/secrets come from the database KV stores. Secret plaintext and
Command-line DSNs may select a database but may not carry password/private-key
credentials; use standard libpq secret mechanisms outside process argv. All
other config/secrets come from the database KV stores. Secret plaintext and
count-tokens prompt content are never accepted as command-line arguments.
"""

Expand All @@ -33,6 +35,9 @@
from contextlib import ExitStack
from typing import List, Optional

from psycopg import ProgrammingError
from psycopg.conninfo import conninfo_to_dict

from . import db
from .batch_api_client import BatchAPIClient, config_credentials_provider
from .bootstrap import resolve_dsn, resolve_secret_key
Expand All @@ -44,6 +49,15 @@
MAX_SECRET_INPUT_CHARACTERS = 65_536
MAX_TOKEN_INPUT_BYTES = 1_048_576
SECRET_LINE_SEPARATORS = frozenset("\n\r\v\f\x1c\x1d\x1e\x85\u2028\u2029")
CLI_DSN_SENSITIVE_PARAMETERS = frozenset(
{
"password",
"passfile",
"sslkey",
"sslpassword",
"oauth_client_secret",
}
)


class _RedactingArgumentParser(argparse.ArgumentParser):
Expand All @@ -59,12 +73,32 @@ def error(self, message: str) -> None:
super().error(redacted_message)


def _validate_cli_dsn(value: str) -> str:
"""Accept valid libpq selectors while refusing credential-bearing argv data."""
try:
parameters = conninfo_to_dict(value)
except ProgrammingError:
raise argparse.ArgumentTypeError(
"Postgres DSN must be valid libpq connection information"
) from None
if CLI_DSN_SENSITIVE_PARAMETERS.intersection(parameters):
raise argparse.ArgumentTypeError(
"Credential-bearing Postgres DSNs are not accepted in --dsn; "
"use libpq secret mechanisms outside process argv"
)
return value


def _add_common(parser: argparse.ArgumentParser) -> None:
"""Add the shared ``--dsn`` option to a subcommand parser."""
"""Add the shared credential-free ``--dsn`` selector to a subcommand parser."""
parser.add_argument(
"--dsn",
default=None,
help="Postgres DSN (else PG_LLM_BATCH_DSN bootstrap env var)",
type=_validate_cli_dsn,
help=(
"Credential-free Postgres selector "
"(else PG_LLM_BATCH_DSN bootstrap env var)"
),
)


Expand Down Expand Up @@ -227,7 +261,7 @@ def build_parser() -> argparse.ArgumentParser:

p_serve = sub.add_parser("serve-healthz", help="Serve GET /healthz")
_add_common(p_serve)
p_serve.add_argument("--host", default="0.0.0.0")
p_serve.add_argument("--host", default="127.0.0.1")
p_serve.add_argument("--port", type=int, default=8080)

return parser
Expand Down
90 changes: 90 additions & 0 deletions tests/test_cli_dsn_argv_security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# SPDX-License-Identifier: Apache-2.0
"""Regression tests for PostgreSQL credential disclosure through CLI argv."""

from __future__ import annotations

import pytest

from pg_llm_batch import cli


@pytest.mark.parametrize(
"credential_dsn",
[
"postgresql://app:secret-sentinel@db.example/batch",
"postgres://app:secret-sentinel@db.example/batch",
"host=db.example dbname=batch user=app password=secret-sentinel",
"host=db.example dbname=batch user=app passfile=/tmp/secret-sentinel.pgpass",
"host=db.example dbname=batch user=app sslkey=/tmp/secret-sentinel.key",
"host=db.example dbname=batch user=app sslpassword=secret-sentinel",
"host=db.example dbname=batch user=app oauth_client_secret=secret-sentinel",
],
)
def test_cli_rejects_credential_bearing_dsn_arguments_without_reflection(
credential_dsn: str,
capsys: pytest.CaptureFixture[str],
) -> None:
"""Credential-bearing DSNs fail in parsing without echoing sensitive argv."""
parser = cli.build_parser()

with pytest.raises(SystemExit):
parser.parse_args(["health", "--dsn", credential_dsn])

captured = capsys.readouterr()
assert "secret-sentinel" not in captured.err
assert "secret-sentinel" not in captured.out


def test_cli_rejects_malformed_dsn_without_reflection(
capsys: pytest.CaptureFixture[str],
) -> None:
"""Malformed conninfo fails with a fixed parser diagnostic."""
parser = cli.build_parser()

with pytest.raises(SystemExit):
parser.parse_args(
["health", "--dsn", "host=db.example password=secret-sentinel broken"]
)

captured = capsys.readouterr()
assert "secret-sentinel" not in captured.err
assert "secret-sentinel" not in captured.out


@pytest.mark.parametrize(
"selector",
[
"postgresql://db.example/batch?sslmode=verify-full",
"host=db.example dbname=batch sslmode=verify-full",
"service=pg-llm-batch",
],
)
def test_cli_retains_credential_free_explicit_database_selectors(selector: str) -> None:
"""Explicit database targeting remains usable when argv contains no secret."""
args = cli.build_parser().parse_args(["health", "--dsn", selector])

assert args.dsn == selector


def test_serve_healthz_cli_defaults_to_loopback() -> None:
"""Direct CLI readiness serving must not bind every host interface by default."""
args = cli.build_parser().parse_args(
["serve-healthz", "--dsn", "postgresql://db.example/batch"]
)

assert args.host == "127.0.0.1"


def test_serve_healthz_cli_allows_explicit_container_binding() -> None:
"""Container callers may deliberately request an all-interface listener."""
args = cli.build_parser().parse_args(
[
"serve-healthz",
"--dsn",
"postgresql://db.example/batch",
"--host",
"0.0.0.0",
]
)

assert args.host == "0.0.0.0"
Loading