fix(server): stop logging the whole environment config when its name is missing

[mlennie]

What this fixes

getProcessedPublisherConfig skips an environment whose name is missing or is not a string, and logged the whole entry alongside the warning: logger.warn(..., { environment }). An Environment carries connections and storageDestinations, so that single line put every credential in that environment into the log. Postgres passwords, BigQuery service-account keys, Snowflake private keys, S3/GCS/Azure keys, whatever the environment declares.

The values are real by the time the warning fires, which is what makes this a leak rather than untidiness. getProcessedPublisherConfig reads through getPublisherConfig, which returns processConfigValue(rawConfig), and that deep-walks the config substituting ${VAR} references. A ${MALLOY_X_PASSWORD} has already become the password before any log line sees it.

Nothing downstream catches it either. redactSensitive is called at the request/response middleware and on the axios-error path, but it is not registered in the winston format chain (createLogger combines only uncolorize, timestamp, errors and json), so logger metadata reaches the transport verbatim.

It also fires at the worst moment: a malformed config is exactly when an operator is reading logs and pasting them into a ticket.

This is the same class as the connection-config leak at connection.ts:1129 that was caught on #736.

The fix

Log the entry's index instead of the entry. The name is precisely what is missing, so position is the only safe thing left to point an operator at the offending entry with.

Deliberately not included

The sibling site in this same file, the missing-name warning in convertConnectionsToApiConnections, is left untouched. It is already fixed in #1071, and changing it here would collide with that PR.

mcp/tools/execute_query_tool.ts logs the MCP tool's params at info level. It carries no credentials so it is not this class, but it does log query and givens, and givens can hold row-level-access identity values. Left for a separate issue rather than folded in, following the scope rule set on #736.

Scope, by method rather than by sample

Every logger.* and console.* call under packages/ was parsed as a balanced-paren span, because the call at issue spans multiple lines and a line-oriented grep cannot see it. Each call's arguments were then classified two ways: a shorthand, spread or key: ident inside an object-literal argument, and a bare identifier passed as a non-first argument.

The second classifier is load-bearing. An earlier pass that lacked it missed 63 sites, and any count taken from it would have been a sample reported as a census.

At this commit that is 884 call sites across 606 files, 549 of them spanning more than one line. 302 put a whole variable into the log. Exactly one of those variables is config-shaped, and it is the convertConnectionsToApiConnections site that #1071 fixes. Before this change there were two. The same script run against the unfixed file reports two, so the before and after are symmetric.

The rest of the credential-leak checklist came back clean. No set -x or bash -x in any of the twelve workflows; secrets are piped to grep -q, redirected into files, or validated with jq ... > /dev/null, and only project_id reaches GITHUB_ENV. No test prints a connection: the credential-adjacent console.* calls are all "Skipping: X credentials not configured" strings. GitHub Actions secret masking is not treated as the mitigation, since winston re-escapes quotes and inserts whitespace, so the masker may not recognise the bytes.

How it was verified

The regression test asserts on the whole logged payload rather than on the absence of an environment key, so re-introducing the config under a different key still fails it.

It was mutation-tested three ways, and each mutation turns it red while restoring returns it green: putting { environment } back, deleting the warning entirely, and dropping the continue so the entry is no longer skipped. So it pins the leak, the warning still firing, and the entry still being skipped. On the first of those the failure output is itself the report, showing the substituted plaintext password inside the logged payload.

Gate on this branch: typecheck clean, lint clean, prettier clean on both files touched. Unit 3217 pass / 3 skip / 0 fail across 152 files. Integration 387 pass / 0 fail across 40 files, and that number needs a caveat: three of those forty files are the bundled storefront example's own tests, collected out of gitignored publisher_data because test:integration passes tests as a substring path filter rather than a directory. The server's own files are 37, and a run against a clean publisher_data reports 315 across 37.

[Sha-Bang]

swapping { environment } for { index } is the right fix for that skip, and the test would catch putting the object back. same file still logs { connection: conn } when a connection has no name — that's the same leak. #1071 changes that line too, but it's a big open PR and these hunks don't overlap; landing this without it still ships the dump. fold the connection log into this one? approving, one inline.

[Sha-Bang]

this is the one. a bit earlier in the same file, convertConnectionsToApiConnections still passes { connection: conn } into logger.warn on a missing name. same substituted credentials, same moment (bad config, someone pasting logs). #1071 rewrites that metadata to { type }, but it doesn't have to wait on that PR — the hunks don't collide. what if we did the same thing here and pinned it the way this test pins the environment dump?