Skip to content

✨ postgres: add PostgreSQL inspection (on-disk + live DB)#7927

Open
tas50 wants to merge 5 commits into
mainfrom
claude/postgresql-inspection-mql-Zc0dF
Open

✨ postgres: add PostgreSQL inspection (on-disk + live DB)#7927
tas50 wants to merge 5 commits into
mainfrom
claude/postgresql-inspection-mql-Zc0dF

Conversation

@tas50

@tas50 tas50 commented May 25, 2026

Copy link
Copy Markdown
Member

Adds two complementary inspection surfaces that both contribute to the
postgresql.* namespace, so a single set of audits can target either
(or both) depending on what credentials are available:

• os provider — on-disk file inspection (no DB creds needed):
postgresql.conf — postgresql.conf with include /
include_dir resolution, plus typed
accessors for the common security
directives (ssl*, password_encryption,
log_*, shared_preload_libraries, ...)
postgresql.hba — pg_hba.conf parsed into a list of rules
with type, database, user,
address, authMethod, options
postgresql.ident — pg_ident.conf mappings

• postgres provider — live-server inspection over libpq (pgx/v5):
postgresql — server root: version, currentDatabase,
currentUser, startedAt, inRecovery,
databases, roles, extensions, settings,
hbaRules, identRules, replicationSlots,
publications, subscriptions, tablespaces,
languages
postgresql.database, postgresql.role, postgresql.extension,
postgresql.setting (pg_settings), postgresql.hbaRule
(pg_hba_file_rules), postgresql.identRule
(pg_ident_file_mappings), postgresql.replicationSlot,
postgresql.publication, postgresql.subscription,
postgresql.tablespace, postgresql.language

The two providers register distinct resources under the same namespace,
so users with only an OS connection can query
postgresql.conf.params, users with only a postgres connection can
query postgresql.databases, and users with both get the union.

Parser logic for postgresql.conf / pg_hba.conf / pg_ident.conf lives in
its own package (providers/os/resources/postgresql) with table-driven
tests covering include resolution, quoted values, inline comments, and
the netmask form of host rules.

https://claude.ai/code/session_018kVL8TQStv9bCfigNNfsjS

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostgreSQL inspection resources may silently return incorrect port (0) on misconfigured servers and retry queries too broadly on version fallback.

Comment thread providers/os/resources/postgresql.go
Comment thread providers/postgres/resources/postgres.go Outdated
Comment thread providers/postgres/resources/postgres.go
Comment thread providers/postgres/provider/provider.go Outdated
Comment thread providers/postgres/resources/postgres.go Outdated
Comment thread providers/postgres/connection/connection.go Outdated
@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Test Results

1 900 tests   1 897 ✅  2m 29s ⏱️
  100 suites      1 💤
    1 files        2 ❌

For more details on these failures, see this check.

Results for commit eccd1dc.

♻️ This comment has been updated with latest results.

@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch from 96046c1 to 3be31e7 Compare May 25, 2026 04:57
@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot dismissed their stale review May 25, 2026 05:00

Superseded by new review

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PostgreSQL inspection feature adds both on-disk config parsing (OS provider) and live DB querying (new postgres provider) — solid overall, with a few correctness and robustness issues.

Comment thread providers/os/resources/postgresql/parser.go Outdated
Comment thread providers/os/resources/postgresql.go
Comment thread providers/os/resources/postgresql.go
Comment thread providers/postgres/resources/postgres.go Outdated
Comment thread providers/postgres/resources/postgres.go Outdated
Comment thread providers/postgres/connection/connection.go Outdated
tas50 added a commit that referenced this pull request May 25, 2026
Address review feedback on PR #7927:

- os/postgresql: fall back to PG default 5432 on bad port parse (port 0
  is never valid).
- postgres/versionNum: drop double round-trip; do one SHOW + ParseInt.
- postgres/hbaRules: narrow the PG < 16 fallback to the missing
  rule_number column instead of swallowing every query error.
- postgres/provider/serverVersion: bound version() with a 30s context
  timeout so a wedged server can't hang Connect().
- postgres/parsePqArray: cur = cur[:0:0] makes the zero-capacity
  re-slice explicit and decouples it from the next append.
- postgres/mergePasswordIntoDSN: strip any pre-existing password= token
  from the libpq k/v DSN before appending the merged one.
- spell check: add 25 PostgreSQL terms (clientcert, csvlog, ctype,
  eventlog, gss, hba, hostgssenc, hostnossl, hostssl, installable,
  jsonlog, ldapserver, libpq, pgcrypto, plpython, Rls, rolinherit,
  samehost, samenet, sameuser, sighup, sspi, tablespace, vartype, wal).

Signed-off-by: Tim Smith <tim@mondoo.com>

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port fallback, HBA query narrowing, and timeout additions look correct; a few prior suggestions remain unaddressed.

Comment thread providers/os/resources/postgresql/parser.go Outdated
Comment thread providers/os/resources/postgresql.go Outdated
Comment thread providers/postgres/resources/postgres.go Outdated
Comment thread providers/postgres/resources/postgres.go Outdated
@github-actions

This comment has been minimized.

tas50 added a commit that referenced this pull request May 25, 2026
Address review feedback on PR #7927:

- os/postgresql: fall back to PG default 5432 on bad port parse (port 0
  is never valid).
- postgres/versionNum: drop double round-trip; do one SHOW + ParseInt.
- postgres/hbaRules: narrow the PG < 16 fallback to the missing
  rule_number column instead of swallowing every query error.
- postgres/provider/serverVersion: bound version() with a 30s context
  timeout so a wedged server can't hang Connect().
- postgres/parsePqArray: cur = cur[:0:0] makes the zero-capacity
  re-slice explicit and decouples it from the next append.
- postgres/mergePasswordIntoDSN: strip any pre-existing password= token
  from the libpq k/v DSN before appending the merged one.
- spell check: add 25 PostgreSQL terms (clientcert, csvlog, ctype,
  eventlog, gss, hba, hostgssenc, hostnossl, hostssl, installable,
  jsonlog, ldapserver, libpq, pgcrypto, plpython, Rls, rolinherit,
  samehost, samenet, sameuser, sighup, sspi, tablespace, vartype, wal).

Signed-off-by: Tim Smith <tsmith84@gmail.com>
@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch from 55389bd to 441da35 Compare May 25, 2026 05:17

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PostgreSQL resources in os provider use current version (13.16.10) in .lr.versions instead of next patch (13.16.11)

Comment thread providers/os/resources/os.lr.versions
Comment thread providers/os/resources/postgresql.go
Comment thread providers/postgres/connection/connection.go Outdated
Comment thread providers/os/resources/postgresql.go
@github-actions

This comment has been minimized.

tas50 added a commit that referenced this pull request May 25, 2026
…ude paths

Round of fixes from the PR #7927 review:

  * postgresql.conf parser canonicalises `path` via `filepath.Clean`
    before checking the cycle guard so equivalent spellings (`./foo`,
    `bar/../foo`, `foo`) collapse to the same key and self-referential
    include loops are caught.
  * `mqlPostgresqlConf.parse()` now guards on a dedicated `parsed bool`
    in the Internal struct instead of overloading `Params.State ==
    StateIsSet`. The old guard never matched the
    `StateIsSet|StateIsNull` value set on error/empty, so a transient
    failure would re-parse on every field access.
  * postgres provider: drop the leftover `var _ = errors.New` stubs in
    `resources/postgres.go` and `connection/connection.go` (and the
    unused `errors` import in connection). `errors.New` is used for
    real elsewhere; the suppression was unnecessary.
  * Replace the hand-rolled `strFromInt` helper with `strconv.Itoa` in
    `postgres.go` and remove the dead function.

The libpq key=value branch of `mergePasswordIntoDSN` already strips
existing `password=` tokens before appending the merged one, and the
URL branch uses `url.UserPassword(...)` which replaces (not appends)
the password — both are correct, no change needed there. The
`hbaRules` fallback still string-matches on `"rule_number"` rather
than type-asserting to a pgx error; refactoring that to inspect the
SQLSTATE on the pgx error is left as a follow-up since we don't
currently import a typed error interface in this file.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostgreSQL config comments now incorrectly describe the escape character, and new resource versions still use the wrong semver.

Comment thread providers/os/resources/postgresql/parser.go Outdated
Comment thread providers/os/resources/os.lr.versions
tas50 added a commit that referenced this pull request May 25, 2026
Round of fixes from the PR #7927 review:

  * `hbaRules` now type-asserts the pgx error to `*pgconn.PgError` and
    checks `Code == "42703"` (undefined_column) instead of string-
    matching on `"rule_number"`. Future error-message wording changes
    in pgx or PostgreSQL won't silently break the PG < 16 fallback.
  * `postgresql.conf` parser comments: the doubled-single-quote escape
    was described with a U+201D right double quotation mark (`”`) in
    the doc-comments — the code itself already handles `''` correctly
    (line-by-line walk in `unquoteConfValue` / `trimInlineComment`),
    only the comment text was misleading. Replaced with `''` so the
    docs match what the parser accepts.
  * Revert the os provider `Version` bump introduced alongside the
    PostgreSQL inspection commit. Feature PRs should not bump
    provider versions — the release flow handles that. The new
    `postgresql.*` entries in `os.lr.versions` are correctly stamped
    at `13.16.10` (the next patch after the shipped `13.16.9`).

The other items the bot re-flagged are already addressed by the prior
commit (7efda8d): the libpq DSN merge strips existing `password=`
tokens before appending; the include-cycle guard keys on
`filepath.Clean(path)`; `parse()` guards on a dedicated `parsed bool`
in the Internal struct; and the `var _ = errors.New` stub is gone.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
@mondoo-code-review mondoo-code-review Bot dismissed their stale review May 25, 2026 06:24

Superseded by new review

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostgreSQL inspection: previous review findings addressed — HBA fallback uses typed error assertion, version numbering corrected, stale lint suppression removed

tas50 added a commit that referenced this pull request May 25, 2026
Address review feedback on PR #7927:

- os/postgresql: fall back to PG default 5432 on bad port parse (port 0
  is never valid).
- postgres/versionNum: drop double round-trip; do one SHOW + ParseInt.
- postgres/hbaRules: narrow the PG < 16 fallback to the missing
  rule_number column instead of swallowing every query error.
- postgres/provider/serverVersion: bound version() with a 30s context
  timeout so a wedged server can't hang Connect().
- postgres/parsePqArray: cur = cur[:0:0] makes the zero-capacity
  re-slice explicit and decouples it from the next append.
- postgres/mergePasswordIntoDSN: strip any pre-existing password= token
  from the libpq k/v DSN before appending the merged one.
- spell check: add 25 PostgreSQL terms (clientcert, csvlog, ctype,
  eventlog, gss, hba, hostgssenc, hostnossl, hostssl, installable,
  jsonlog, ldapserver, libpq, pgcrypto, plpython, Rls, rolinherit,
  samehost, samenet, sameuser, sighup, sspi, tablespace, vartype, wal).

Signed-off-by: Tim Smith <tsmith84@gmail.com>
tas50 added a commit that referenced this pull request May 25, 2026
…ude paths

Round of fixes from the PR #7927 review:

  * postgresql.conf parser canonicalises `path` via `filepath.Clean`
    before checking the cycle guard so equivalent spellings (`./foo`,
    `bar/../foo`, `foo`) collapse to the same key and self-referential
    include loops are caught.
  * `mqlPostgresqlConf.parse()` now guards on a dedicated `parsed bool`
    in the Internal struct instead of overloading `Params.State ==
    StateIsSet`. The old guard never matched the
    `StateIsSet|StateIsNull` value set on error/empty, so a transient
    failure would re-parse on every field access.
  * postgres provider: drop the leftover `var _ = errors.New` stubs in
    `resources/postgres.go` and `connection/connection.go` (and the
    unused `errors` import in connection). `errors.New` is used for
    real elsewhere; the suppression was unnecessary.
  * Replace the hand-rolled `strFromInt` helper with `strconv.Itoa` in
    `postgres.go` and remove the dead function.

The libpq key=value branch of `mergePasswordIntoDSN` already strips
existing `password=` tokens before appending the merged one, and the
URL branch uses `url.UserPassword(...)` which replaces (not appends)
the password — both are correct, no change needed there. The
`hbaRules` fallback still string-matches on `"rule_number"` rather
than type-asserting to a pgx error; refactoring that to inspect the
SQLSTATE on the pgx error is left as a follow-up since we don't
currently import a typed error interface in this file.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
tas50 added a commit that referenced this pull request May 25, 2026
Round of fixes from the PR #7927 review:

  * `hbaRules` now type-asserts the pgx error to `*pgconn.PgError` and
    checks `Code == "42703"` (undefined_column) instead of string-
    matching on `"rule_number"`. Future error-message wording changes
    in pgx or PostgreSQL won't silently break the PG < 16 fallback.
  * `postgresql.conf` parser comments: the doubled-single-quote escape
    was described with a U+201D right double quotation mark (`”`) in
    the doc-comments — the code itself already handles `''` correctly
    (line-by-line walk in `unquoteConfValue` / `trimInlineComment`),
    only the comment text was misleading. Replaced with `''` so the
    docs match what the parser accepts.
  * Revert the os provider `Version` bump introduced alongside the
    PostgreSQL inspection commit. Feature PRs should not bump
    provider versions — the release flow handles that. The new
    `postgresql.*` entries in `os.lr.versions` are correctly stamped
    at `13.16.10` (the next patch after the shipped `13.16.9`).

The other items the bot re-flagged are already addressed by the prior
commit (7efda8d): the libpq DSN merge strips existing `password=`
tokens before appending; the include-cycle guard keys on
`filepath.Clean(path)`; `parse()` guards on a dedicated `parsed bool`
in the Internal struct; and the `var _ = errors.New` stub is gone.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch from 5198d06 to 1ec8699 Compare May 25, 2026 06:25
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Querying replication slots will fail on PostgreSQL 12–14 due to missing columns.

Comment thread providers/postgres/resources/postgres.go
Comment thread providers/postgres/resources/postgres.go
Comment thread providers/postgres/resources/postgres.go
@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch 2 times, most recently from cc4cb93 to 1ec8699 Compare May 25, 2026 16:42
@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot dismissed their stale review May 25, 2026 16:44

Superseded by new review

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PostgreSQL on-disk inspection resources and live DB provider are well-structured; one minor non-determinism in results ordering.

Comment thread providers/os/resources/postgresql.go

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PostgreSQL inspection resources for on-disk config and live DB queries are well-structured overall, with a few correctness and security items to address.

Comment thread providers/postgres/config/config.go
Comment thread providers/postgres/resources/postgres.go
Comment thread providers/postgres/resources/postgres.go
Comment thread providers/os/resources/postgresql.go
tas50 added a commit that referenced this pull request May 25, 2026
Address review feedback on PR #7927:

- os/postgresql: fall back to PG default 5432 on bad port parse (port 0
  is never valid).
- postgres/versionNum: drop double round-trip; do one SHOW + ParseInt.
- postgres/hbaRules: narrow the PG < 16 fallback to the missing
  rule_number column instead of swallowing every query error.
- postgres/provider/serverVersion: bound version() with a 30s context
  timeout so a wedged server can't hang Connect().
- postgres/parsePqArray: cur = cur[:0:0] makes the zero-capacity
  re-slice explicit and decouples it from the next append.
- postgres/mergePasswordIntoDSN: strip any pre-existing password= token
  from the libpq k/v DSN before appending the merged one.
- spell check: add 25 PostgreSQL terms (clientcert, csvlog, ctype,
  eventlog, gss, hba, hostgssenc, hostnossl, hostssl, installable,
  jsonlog, ldapserver, libpq, pgcrypto, plpython, Rls, rolinherit,
  samehost, samenet, sameuser, sighup, sspi, tablespace, vartype, wal).

Signed-off-by: Tim Smith <tsmith84@gmail.com>
tas50 added a commit that referenced this pull request May 25, 2026
…ude paths

Round of fixes from the PR #7927 review:

  * postgresql.conf parser canonicalises `path` via `filepath.Clean`
    before checking the cycle guard so equivalent spellings (`./foo`,
    `bar/../foo`, `foo`) collapse to the same key and self-referential
    include loops are caught.
  * `mqlPostgresqlConf.parse()` now guards on a dedicated `parsed bool`
    in the Internal struct instead of overloading `Params.State ==
    StateIsSet`. The old guard never matched the
    `StateIsSet|StateIsNull` value set on error/empty, so a transient
    failure would re-parse on every field access.
  * postgres provider: drop the leftover `var _ = errors.New` stubs in
    `resources/postgres.go` and `connection/connection.go` (and the
    unused `errors` import in connection). `errors.New` is used for
    real elsewhere; the suppression was unnecessary.
  * Replace the hand-rolled `strFromInt` helper with `strconv.Itoa` in
    `postgres.go` and remove the dead function.

The libpq key=value branch of `mergePasswordIntoDSN` already strips
existing `password=` tokens before appending the merged one, and the
URL branch uses `url.UserPassword(...)` which replaces (not appends)
the password — both are correct, no change needed there. The
`hbaRules` fallback still string-matches on `"rule_number"` rather
than type-asserting to a pgx error; refactoring that to inspect the
SQLSTATE on the pgx error is left as a follow-up since we don't
currently import a typed error interface in this file.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
tas50 added a commit that referenced this pull request May 25, 2026
Round of fixes from the PR #7927 review:

  * `hbaRules` now type-asserts the pgx error to `*pgconn.PgError` and
    checks `Code == "42703"` (undefined_column) instead of string-
    matching on `"rule_number"`. Future error-message wording changes
    in pgx or PostgreSQL won't silently break the PG < 16 fallback.
  * `postgresql.conf` parser comments: the doubled-single-quote escape
    was described with a U+201D right double quotation mark (`”`) in
    the doc-comments — the code itself already handles `''` correctly
    (line-by-line walk in `unquoteConfValue` / `trimInlineComment`),
    only the comment text was misleading. Replaced with `''` so the
    docs match what the parser accepts.
  * Revert the os provider `Version` bump introduced alongside the
    PostgreSQL inspection commit. Feature PRs should not bump
    provider versions — the release flow handles that. The new
    `postgresql.*` entries in `os.lr.versions` are correctly stamped
    at `13.16.10` (the next patch after the shipped `13.16.9`).

The other items the bot re-flagged are already addressed by the prior
commit (7efda8d): the libpq DSN merge strips existing `password=`
tokens before appending; the include-cycle guard keys on
`filepath.Clean(path)`; `parse()` guards on a dedicated `parsed bool`
in the Internal struct; and the `var _ = errors.New` stub is gone.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch from 1ec8699 to 8674942 Compare May 25, 2026 16:46
@github-actions

This comment has been minimized.

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New PostgreSQL on-disk config inspection and live DB provider for auditing server configuration, authentication rules, roles, and extensions.

Comment thread providers/postgres/resources/postgres.go
Comment thread providers/postgres/resources/postgres.go Outdated
tas50 and others added 5 commits June 3, 2026 10:01
Adds two complementary inspection surfaces that both contribute to the
`postgresql.*` namespace, so a single set of audits can target either
(or both) depending on what credentials are available:

  • os provider — on-disk file inspection (no DB creds needed):
      postgresql.conf      — postgresql.conf with `include` /
                              `include_dir` resolution, plus typed
                              accessors for the common security
                              directives (`ssl*`, `password_encryption`,
                              `log_*`, `shared_preload_libraries`, ...)
      postgresql.hba       — pg_hba.conf parsed into a list of rules
                              with `type`, `database`, `user`,
                              `address`, `authMethod`, `options`
      postgresql.ident     — pg_ident.conf mappings

  • postgres provider — live-server inspection over libpq (pgx/v5):
      postgresql           — server root: version, currentDatabase,
                              currentUser, startedAt, inRecovery,
                              databases, roles, extensions, settings,
                              hbaRules, identRules, replicationSlots,
                              publications, subscriptions, tablespaces,
                              languages
      postgresql.database, postgresql.role, postgresql.extension,
      postgresql.setting (pg_settings), postgresql.hbaRule
      (pg_hba_file_rules), postgresql.identRule
      (pg_ident_file_mappings), postgresql.replicationSlot,
      postgresql.publication, postgresql.subscription,
      postgresql.tablespace, postgresql.language

The two providers register distinct resources under the same namespace,
so users with only an OS connection can query
`postgresql.conf.params`, users with only a postgres connection can
query `postgresql.databases`, and users with both get the union.

Parser logic for postgresql.conf / pg_hba.conf / pg_ident.conf lives in
its own package (providers/os/resources/postgresql) with table-driven
tests covering include resolution, quoted values, inline comments, and
the netmask form of host rules.

https://claude.ai/code/session_018kVL8TQStv9bCfigNNfsjS
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Address review feedback on PR #7927:

- os/postgresql: fall back to PG default 5432 on bad port parse (port 0
  is never valid).
- postgres/versionNum: drop double round-trip; do one SHOW + ParseInt.
- postgres/hbaRules: narrow the PG < 16 fallback to the missing
  rule_number column instead of swallowing every query error.
- postgres/provider/serverVersion: bound version() with a 30s context
  timeout so a wedged server can't hang Connect().
- postgres/parsePqArray: cur = cur[:0:0] makes the zero-capacity
  re-slice explicit and decouples it from the next append.
- postgres/mergePasswordIntoDSN: strip any pre-existing password= token
  from the libpq k/v DSN before appending the merged one.
- spell check: add 25 PostgreSQL terms (clientcert, csvlog, ctype,
  eventlog, gss, hba, hostgssenc, hostnossl, hostssl, installable,
  jsonlog, ldapserver, libpq, pgcrypto, plpython, Rls, rolinherit,
  samehost, samenet, sameuser, sighup, sspi, tablespace, vartype, wal).

Signed-off-by: Tim Smith <tsmith84@gmail.com>
…ude paths

Round of fixes from the PR #7927 review:

  * postgresql.conf parser canonicalises `path` via `filepath.Clean`
    before checking the cycle guard so equivalent spellings (`./foo`,
    `bar/../foo`, `foo`) collapse to the same key and self-referential
    include loops are caught.
  * `mqlPostgresqlConf.parse()` now guards on a dedicated `parsed bool`
    in the Internal struct instead of overloading `Params.State ==
    StateIsSet`. The old guard never matched the
    `StateIsSet|StateIsNull` value set on error/empty, so a transient
    failure would re-parse on every field access.
  * postgres provider: drop the leftover `var _ = errors.New` stubs in
    `resources/postgres.go` and `connection/connection.go` (and the
    unused `errors` import in connection). `errors.New` is used for
    real elsewhere; the suppression was unnecessary.
  * Replace the hand-rolled `strFromInt` helper with `strconv.Itoa` in
    `postgres.go` and remove the dead function.

The libpq key=value branch of `mergePasswordIntoDSN` already strips
existing `password=` tokens before appending the merged one, and the
URL branch uses `url.UserPassword(...)` which replaces (not appends)
the password — both are correct, no change needed there. The
`hbaRules` fallback still string-matches on `"rule_number"` rather
than type-asserting to a pgx error; refactoring that to inspect the
SQLSTATE on the pgx error is left as a follow-up since we don't
currently import a typed error interface in this file.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
Round of fixes from the PR #7927 review:

  * `hbaRules` now type-asserts the pgx error to `*pgconn.PgError` and
    checks `Code == "42703"` (undefined_column) instead of string-
    matching on `"rule_number"`. Future error-message wording changes
    in pgx or PostgreSQL won't silently break the PG < 16 fallback.
  * `postgresql.conf` parser comments: the doubled-single-quote escape
    was described with a U+201D right double quotation mark (`”`) in
    the doc-comments — the code itself already handles `''` correctly
    (line-by-line walk in `unquoteConfValue` / `trimInlineComment`),
    only the comment text was misleading. Replaced with `''` so the
    docs match what the parser accepts.
  * Revert the os provider `Version` bump introduced alongside the
    PostgreSQL inspection commit. Feature PRs should not bump
    provider versions — the release flow handles that. The new
    `postgresql.*` entries in `os.lr.versions` are correctly stamped
    at `13.16.10` (the next patch after the shipped `13.16.9`).

The other items the bot re-flagged are already addressed by the prior
commit (7efda8d): the libpq DSN merge strips existing `password=`
tokens before appending; the include-cycle guard keys on
`filepath.Clean(path)`; `parse()` guards on a dedicated `parsed bool`
in the Internal struct; and the `var _ = errors.New` stub is gone.

Signed-off-by: Tim Smith <tsmith84@gmail.com>
Rebase onto origin/main introduced conflicts in os.lr (squid+haproxy vs
postgresql resources) and os.lr.go (generated). Resolved by keeping both
resource sets; regenerated os.lr.go. Fixed missing closing brace for
haproxy.config.peersSection that was lost during conflict resolution.
Renamed local paramString in postgresql.go to avoid symbol clash with
the variadic version added to rsyslog_parse.go in main.

Address two unresolved review comments:
- replicationSlots(): add graceful fallback for PG < 14 (two_phase column)
  and PG < 13 (wal_status column) using the same undefined_column (42703)
  pattern used by hbaRules().
- subscriptions(): redact password= segments from connInfo before storing
  so plaintext credentials don't surface in scan results.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tas50 tas50 force-pushed the claude/postgresql-inspection-mql-Zc0dF branch from 8674942 to eccd1dc Compare June 3, 2026 17:10
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (1)

postgres

These words are not needed and should be removed CEPHFS cephx clientcert cpx CRDR cribl crowdstrike cryptokey Csec CSEK csvlog ctype customfield customfieldtypes customresources cwes dapr dast databricks evc eventarc eventhubs eventlog Eventstream evpn evs EXACC exadata exo GLUSTERFS groupname Groupsv grpcroute gss gsuite gua gvnic hadoop hba HCMCLOUD hdd headerorder hec hil Hns hookscript horizontalpodautoscaler hostedzone hostgssenc hostkeyalgorithms hostnossl hostpci hostssl iap ibpb iccid identitycenter identitysource idps IKEv ilb imds imei ingresstls initrdefi INPROGRESS installable INSTALLTIME jsd jsonbody jsonlog julia junie kty kubenet kustomization KVM labelmatchstatement lastname launchconfiguration launchtemplate ldapmain ldapserver lfs libpq limitrange linkedservices linklayer MAPRFS parallelquery pbs pcnet PDB pdp persistentvolume persistentvolumeclaim pfs pgcrypto pgp phpipam pipefail Pixtral pki plpython poddisruptionbudget recordsets recrawl referencegrant regexmatchstatement regexpatternsetreferencestatement Remediator renv RESKEY resourcegroup resourcepolicy resourcequota revalidates richrule Rls roku rolearn rolinherit rootdir rpo RRULE RSASHA RSASSA RSession Rtt RTX rubygems rulegroup rulegroupreferencestatement rxtx saas safetensors samehost samenet samesite sameuser sas sasl savedmodel sbfm sbom scc scim scm sdn sdp serviceconnection servicedesk serviceprincipals SEV Sflags sfn sfo sgp sighup signin Signout sigv singlequeryargument sizeconstraintstatement skaffold slaac splunk spo sqli sqlimatchstatement sqlserver sriov srp ssbd SSHFP ssis sslv sspi Sspr stan tablespace tacacs tailnets Uocm upn uptycs usb Usec userdata USERPOOL userspace Utc valkey vartype VAULTNAME VCores vdcs vdev vdi vdso verifiedaccess vertexai Veth vga VGeneration vlan vxlan vzdump vztmpl WAFV wal Wasi webauthn webide webservers websockets Webstore whl wifi

To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the git@github.com:mondoohq/mql.git repository
on the claude/postgresql-inspection-mql-Zc0dF branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c/apply.pl' |
perl - 'https://github.com/mondoohq/mql/actions/runs/26900714435/attempts/1' &&
git commit -m 'Update check-spelling metadata'
Available 📚 dictionaries could cover words (expected and unrecognized) not in the 📘 dictionary

This includes both expected items (408) from .github/actions/spelling/expect.txt and unrecognized words (1)

Dictionary Entries Covers Uniquely
cspell:node/dict/node.txt 891 5 2
cspell:python/src/python/python-lib.txt 2417 4
cspell:dotnet/dict/dotnet.txt 405 3 2
cspell:django/dict/django.txt 393 3 1
cspell:python/src/python/python.txt 392 3

Consider adding them (in .github/workflows/spell-check.yaml) in jobs:/spelling: for uses: check-spelling/check-spelling@cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c in its with to extra_dictionaries:

            cspell:node/dict/node.txt
            cspell:python/src/python/python-lib.txt
            cspell:dotnet/dict/dotnet.txt
            cspell:django/dict/django.txt
            cspell:python/src/python/python.txt

To stop checking additional dictionaries, add (in .github/workflows/spell-check.yaml) for uses: check-spelling/check-spelling@cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c in its with:

check_extra_dictionaries: ""
Warnings ⚠️ (1)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

⚠️ Warnings Count
⚠️ ignored-expect-variant 1

See ⚠️ Event descriptions for more information.

If the flagged items are false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Password redaction for subscription connection strings can leak partial credentials when passwords contain single quotes.

Comment on lines +928 to +934
end := strings.Index(out[start:], "'")
if end < 0 {
// Unclosed quote — redact to end of string.
out = out[:idx] + "password=***"
break
}
out = out[:idx] + "password=***" + out[start+end+1:]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 criticalredactConnInfo doesn't handle backslash-escaped quotes inside single-quoted passwords. libpq encodes password='pa\'ss' and this code stops at the escaped ' (position of \'), producing password=***ss' host=... — leaking the tail of the password into scan results stored in the connInfo field of postgresql.subscription.

Fix: walk the string respecting \' escapes, same as libpqUnquote already does:

// Find closing quote, skipping \' escapes
end := -1
for j := start; j < len(out); j++ {
    if out[j] == '\\' && j+1 < len(out) {
        j++ // skip escaped char
        continue
    }
    if out[j] == '\'' {
        end = j - start
        break
    }
}

}
// Bare-word form: password=... (value ends at next space or end of string)
for {
idx := strings.Index(out, "password=")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 warningstrings.Index(out, "password=") matches the substring anywhere, so a hypothetical DSN key like sslpassword=foo would be matched first, causing the real password=secret to remain unredacted. Add a word-boundary check (beginning of string or preceded by a space):

if idx > 0 && out[idx-1] != ' ' {
    // not a standalone key — skip past it
    out = out[idx+len("password="):]
    continue
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant