fix(agent): keep passwd file content out of parse errors

[otavio]

Closes code scanning alerts #195, #196, #198, #240, #300 (go/clear-text-logging).

The leak

parsePasswdLine, parseGroupLine and the FreeBSD parseMasterPasswdLine built their errors by interpolating the field they had just failed to parse:

return result, fmt.Errorf("passwd line had badly formatted uid %s", parts[2])

Every caller logs that error with WithError(err), so a field that doesn't parse as a number is copied verbatim into the agent's log.

In host mode the file is the host's own /etc/passwd — untidy, no more. In connector mode it is /etc/passwd read out of the container being attached to (getPasswd → docker cp), which the image author controls. A uid field of $(...)\n level=fatal msg=... lands in the log as written. Whoever ships those logs onward — anything parsing logfmt, anything rendering them in a browser — pays for it.

No password ever reached a log; CodeQL's "sensitive data" is the passwd bytes themselves. The reachable problem is log injection from a hostile image, which is why this is worth fixing rather than dismissing.

The fix

The field's content was never the useful part of the message — its position was. Line parsers now return errMalformedUID / errMalformedGID, and the readers wrap with the line number they already track:

passwd line 12: malformed uid field

Field-count mismatches keep their counts: len(parts) is a number the parser derived, not bytes from the file.

Also

Two log calls in the connector authenticator passed WithError(err) where err is provably nil — both sit after an if err != nil { ...; return false }, so they only ever recorded error: null. One of them is the sink CodeQL reports as #300, meaning the reported leak was of a nil. Dropped both.

Testing

TestParseErrorsDoNotEchoFileContent feeds a marker through the uid, gid and field-count paths of the passwd, group and shadow readers, asserting the error names the line but not the content. Mutation-checked — restoring one interpolation turns it red:

"passwd line 2: badly formatted uid SUPERSECRET-injected-by-a-hostile-image"
  should not contain "SUPERSECRET-injected-by-a-hostile-image"

Full agent suite green under -tags docker -race; gofmt and golangci-lint clean.

One thing I could not verify

GOOS=freebsd does not build on master either, for an unrelated pre-existing reason — backend.ListGroups returns []string where Backend wants []uint32 (auth_freebsd.go:45). FreeBSD isn't in the CI matrix, so nothing catches it. auth_freebsd.go is changed here to match the other parsers, but it could not be compiled to prove it. Happy to open a separate issue for the FreeBSD backend.

[github-actions]

Claude finished @otavio's task in 2m 21s —— View job

---

Code Review Complete

Reviewed 5 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

A few things I verified while reviewing:

• Leak actually closed. The uid/gid parse errors now return the content-free sentinels errMalformedUID/errMalformedGID, and the field-count errors interpolate only len(parts) (a derived int, not file bytes). No remaining error/log path in agent/pkg/osauth interpolates a parts[...] field or raw line. As a bonus, discarding the raw strconv error also drops its parsing "…" echo, which was a secondary leak vector.
• Line numbering is right. for lineno := 1; ; lineno++ increments on every physical line including continued blank/comment lines, so the reported line matches the file — and each wantLine assertion in the test checks out.
• The two dropped WithError(err) calls were provably on nil. At authenticator.go:103 and :126 the in-scope err came from calls already nil-guarded upstream (AuthUserFromShadow returns only a bool), so nothing real was discarded.
• No cross-repo impact. The changed error strings/values live entirely inside osauth; the sentinels are unexported, ErrUserNotFound is unchanged, and cloud/ does not import this package.
• Test is mutation-sensitive — reverting an interpolation turns it red, as claimed.

On the FreeBSD note in the PR description: auth_freebsd.go mirrors the tested Linux paths but isn't compiled in CI, so those changes remain unverified by build/test. That's a pre-existing CI-matrix constraint, not something this PR introduces — a separate issue for the backend.ListGroups []string vs []uint32 mismatch sounds like the right call.

---

To request another review round, comment /review.