Skip to content

Commit 7efda8d

Browse files
committed
🐛 postgres: clean parse guard, dedupe DSN password, canonicalise include 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>
1 parent 441da35 commit 7efda8d

4 files changed

Lines changed: 22 additions & 38 deletions

File tree

providers/os/resources/postgresql.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ var postgresqlConfPaths = []string{
4747

4848
type mqlPostgresqlConfInternal struct {
4949
lock sync.Mutex
50+
// parsed flips to true once parse() has run to completion (whether the
51+
// outcome was data, empty, or an error). It's a dedicated flag rather
52+
// than overloading `s.Params.State` because the empty- and error-paths
53+
// set extra state bits (StateIsNull) that the previous equality guard
54+
// failed to recognise as "already parsed", causing infinite re-parses.
55+
parsed bool
5056
}
5157

5258
func initPostgresqlConf(runtime *plugin.Runtime, args map[string]*llx.RawData) (map[string]*llx.RawData, plugin.Resource, error) {
@@ -104,9 +110,13 @@ func (s *mqlPostgresqlConf) file() (*mqlFile, error) {
104110
func (s *mqlPostgresqlConf) parse(file *mqlFile) error {
105111
s.lock.Lock()
106112
defer s.lock.Unlock()
107-
if s.Params.State == plugin.StateIsSet {
113+
if s.parsed {
108114
return nil
109115
}
116+
// Flip the guard before any early-return path so transient empty/error
117+
// outcomes don't trigger an infinite re-parse loop on subsequent field
118+
// accesses.
119+
s.parsed = true
110120

111121
if file == nil {
112122
s.setConfEmpty()

providers/os/resources/postgresql/parser.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,15 @@ func ParseConf(path string, fileReader FileReader, dirLister func(dir string) ([
5353
}
5454

5555
func parseConfRec(c *Conf, path string, fileReader FileReader, dirLister func(dir string) ([]string, error), visited map[string]bool) error {
56-
abs := path
57-
if visited[abs] {
56+
// Canonicalise the path before checking the cycle guard so equivalent
57+
// spellings (`./foo.conf` vs `conf.d/../foo.conf` vs `foo.conf`) collapse
58+
// to the same key. Without this the recursive include detection would
59+
// miss self-referential loops that walk through `..` segments.
60+
key := filepath.Clean(path)
61+
if visited[key] {
5862
return nil
5963
}
60-
visited[abs] = true
64+
visited[key] = true
6165

6266
content, err := fileReader(path)
6367
if err != nil {
@@ -128,7 +132,7 @@ func parseConfRec(c *Conf, path string, fileReader FileReader, dirLister func(di
128132
// splitConfKV parses a single non-comment, non-blank postgresql.conf line
129133
// into a key and a value. PostgreSQL accepts either `key = value` or
130134
// `key value` (the `=` is optional). The value may be a bare token, a
131-
// number with a unit suffix, or a single-quoted string with `''` for an
135+
// number with a unit suffix, or a single-quoted string with `` for an
132136
// embedded single quote.
133137
func splitConfKV(line string) (string, string, bool) {
134138
// Find the end of the key — first whitespace or '='
@@ -149,7 +153,7 @@ func splitConfKV(line string) (string, string, bool) {
149153
}
150154

151155
// unquoteConfValue strips surrounding single quotes from a postgresql.conf
152-
// value and resolves the `''` doubled-quote escape inside the string.
156+
// value and resolves the `` doubled-quote escape inside the string.
153157
// Returned unchanged when the value isn't quoted.
154158
func unquoteConfValue(s string) string {
155159
if len(s) < 2 || s[0] != '\'' {

providers/postgres/connection/connection.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package connection
66
import (
77
"context"
88
"database/sql"
9-
"errors"
109
"net"
1110
"net/url"
1211
"strconv"
@@ -341,6 +340,3 @@ func (c *PostgresConnection) HostPort() string {
341340
}
342341
return net.JoinHostPort(host, strconv.Itoa(port))
343342
}
344-
345-
// Ensure unused error variable does not break import.
346-
var _ = errors.New

providers/postgres/resources/postgres.go

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ import (
1616
"go.mondoo.com/mql/v13/types"
1717
)
1818

19-
// silence unused-import linting in case we trim later
20-
var _ = errors.New
21-
2219
// id returns the cache key for the singleton postgresql resource. The asset
2320
// identity already encodes the host/port/database, so a static string is
2421
// stable and unique within the runtime.
@@ -808,32 +805,9 @@ func stringsToAny(in []string) []any {
808805
// blank (e.g. an error row from pg_hba_file_rules with no line context).
809806
func file_lineID(resource, file string, line int64, idx int) string {
810807
if file == "" {
811-
return resource + "/idx/" + strFromInt(idx)
812-
}
813-
return resource + "/" + file + ":" + strFromInt(int(line)) + "/" + strFromInt(idx)
814-
}
815-
816-
func strFromInt(i int) string {
817-
if i == 0 {
818-
return "0"
819-
}
820-
neg := false
821-
if i < 0 {
822-
neg = true
823-
i = -i
824-
}
825-
var buf [20]byte
826-
pos := len(buf)
827-
for i > 0 {
828-
pos--
829-
buf[pos] = byte('0' + i%10)
830-
i /= 10
831-
}
832-
if neg {
833-
pos--
834-
buf[pos] = '-'
808+
return resource + "/idx/" + strconv.Itoa(idx)
835809
}
836-
return string(buf[pos:])
810+
return resource + "/" + file + ":" + strconv.Itoa(int(line)) + "/" + strconv.Itoa(idx)
837811
}
838812

839813
// optionsListToMap turns the libpq-style ["key=value", "flag"] options array

0 commit comments

Comments
 (0)