fix: mask get_application credentials and project the server row

[StuMason]

Closes #332. Same treatment as #331: mask the #209 sensitive-field list on get_application / updateApplication / non-summary listApplications with reveal: true opt-in on get, and project embedded destination.server rows to the summary shape unconditionally. Four new tests with never-contains-secret assertions; get_application contract snapshot regenerated. Found live: reading an app's config through the MCP returned the deploy-webhook HMAC keys and the server's log-drain credentials in the clear.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.67%. Comparing base (c39d415) to head (928e001).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   93.64%   93.67%   +0.02%     
==========================================
  Files           4        4              
  Lines         740      743       +3     
  Branches      205      205              
==========================================
+ Hits          693      696       +3     
  Misses          6        6              
  Partials       41       41              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Review: mask get_application credentials and project the server row

Static review — I could not run npm test / npm run build in this sandbox, so nothing below is a test-run result.

The core change is right and cheap: it reuses the sanitizeResourceDetail / projectNestedServers machinery from #328 rather than growing a parallel masker, so get_application inherits the same field list, the same reveal ergonomics and the same never-brings-the-server-back guarantee as get_database / get_service. The apps.map((app) => sanitizeResourceDetail(app)) arrow at coolify-client.ts:1163 is the right call too — a bare .map(sanitizeResourceDetail) would pass the array index in as reveal. Snapshot matches the get_database shape exactly. Asserting on JSON.stringify(result) rather than named fields is the correct instinct for this class of bug.

Findings, roughly by impact.

1. The read-side mask makes a write-side round trip destructive

application update forwards everything the model hands it:

const { action: _, uuid: __, delete_volumes: ___, ...updateData } = args;
return wrap(() => this.client.updateApplication(uuid, updateData));

(mcp-server.ts:1340)

A model that calls get_application, edits one field and echoes the object back now sends custom_labels: '***' and docker_compose_raw: '***'. Both are accepted:

• custom_labels: '***' overwrites the Traefik label block — routing rules and basic-auth htpasswd entries, silently.
• docker_compose_raw goes through toBase64 at coolify-client.ts:1235-1237, so '***' becomes Kioq and the app's compose file becomes the literal string ***.

This hazard technically arrived with #331 for updateService / updateDatabase, but it lands harder here: update is the most-used action on the application tool, and its handler spreads the whole arg object rather than picking fields. Since the read path is what puts *** into the model's context, closing it belongs with this change.

Cheapest fix is a dropMaskedFields(payload) over SENSITIVE_RESOURCE_FIELDS on the write paths — dropping is the safe direction (a dropped field is a no-op; a written *** is data loss). Erroring with "this value is masked; re-read with reveal: true if you meant to rewrite it" is also defensible and more legible to the model. Either way it wants a test.

2. CHANGELOG.md and README.md are not updated

CLAUDE.md's Documentation Standards require both in the same PR, and #331 did exactly that (CHANGELOG.md +1/-1, README.md +5). Concretely:

• README.md:115 — the "Secure by default" bullet names get_database / get_service; it should name get_application too, and mention that the non-summary list_applications path and update responses are masked as well.
• CHANGELOG.md — 2.19.2 shipped today as a security release and this is the same class of fix; it needs its own ### Security entry under a new version.

3. The new live payload fact is not recorded in the types

Application['destination'] (src/types/coolify.ts:249-254) declares only id / uuid / name / server_id — no server. The PR body says this was found live (log-drain credentials came back through it), so /applications/{uuid} nests a full server row there, and right now the only record of that is a test fixture. The surrounding block already documents exactly this kind of verified-live shape detail for server_id; adding server?: Server with a note that it is projected down to a summary would keep that file the source of truth. It is also why the projection currently only compiles through the Record<string, unknown> cast.

4. Test gap: destination.server_id survival

leakyApplication.destination carries id and network but no server_id, and the assertion only checks destination.server. server_id is what stop_all_apps groups by (mcp-server.ts ~3135), and CLAUDE.md records that keying off the wrong field there already shipped as dead code once, plus the server_id: 0 falsiness trap. projectNestedServers does preserve it — it spreads destination and replaces only .server — but a fixture with server_id: 0 plus an assertion that it survives the projection would pin the field that has historically broken. Same argument for a top-level server key on the app row, which the helper also handles but nothing exercises for applications.

5. updateServiceApplication is the one application-typed response left unmasked

coolify-client.ts:1599-1612 returns Promise<Application> straight from PATCH /services/{uuid}/applications/{uuid} with no sanitizeResourceDetail. Upstream's ServiceApplication model is not the same table as Application, so it may genuinely carry none of these fields — worth a live check against 4.1.2 rather than assuming either way, given the type here claims it is an Application.

6. Nothing in evals/ covers reveal at all

grep -rn reveal evals/src outside the snapshots returns nothing. Tool descriptions are prompts, and this PR adds a fourth "pass reveal: true when you explicitly need them" surface — the failure mode is a model that reflexively sets it for a benign "show me the app config". That is a suite-wide gap (env_vars, get_database, get_service, now get_application), not this PR's debt, but this PR is what makes it worth writing.

Smaller notes

• diagnoseApplication calls getApplication(uuid) with no options, so diagnostics get the masked row. Correct default, worth nothing more than a nod.
• The description lists "compose bodies, labels" while the shared field list also masks db passwords and internal/external_db_url — inert on an application row, so no action, just noting the description and the mask are not 1:1.
• The declared return type stays Promise<Application> while destination.server is now a ServerSummary. Consistent with how get_database / get_service raw passthrough includes plaintext credentials #328 left Database / Service, so no objection, but item 3 above would let the type say what is actually returned.

Performance impact is negligible: two shallow spreads per row on the full listApplications path, and every internal consumer (resolveApplicationUuid, applicationsInProject, projectContents, stopAllApps, the GitHub-app delete count) reads only scalars that survive both the mask and the projection — I checked each one.

None of these undercut the fix. Item 1 is the only one I would want resolved before merge, and item 2 is required by the repo's own conventions.

[claude]

Review — #333 (mask get_application, project the server row)

Good, tightly-scoped continuation of #327/#328. Reusing sanitizeResourceDetail rather than growing a second masker is the right call, the tests assert on the serialized result (not.toContain('sentinel-secret')) rather than on individual keys, the null-payload test covers the defensive arms, and the contract snapshot was regenerated. I traced every internal caller of the three touched methods — applicationsInProject, projectContents, findInfrastructureIssues, bulkEnvUpdate's name lookup, the GitHub-app delete count (source_id/source_type), stopAllApps (destination.server_id, which survives the projection since only destination.server is replaced) and the delete-confirmation getApplication (app.name) — none read a masked or projected field, so nothing regresses behind this. I did not run the suite; the sandbox blocked npx.

Findings below, roughly by how much they'd cost.

1. A read-modify-write can now persist the literal *** (medium)

get_application masks custom_labels and docker_compose_raw, and the application tool's update action forwards every arg it receives straight through (src/lib/mcp-server.ts:1339-1341 — const { action, uuid, delete_volumes, ...updateData } = args). So the classic agent loop "read the app, change one field, write it back" ends with custom_labels: '***' on the wire, and Coolify happily stores it — Traefik basic-auth labels replaced by three asterisks, or a compose body destroyed. docker_compose_raw is worse in one respect: the client base64-encodes it on the way out, so the stored value is a valid-looking blob.

There is no write-side sentinel guard anywhere in the client today (I grepped MASKED_VALUE — it is only ever written, never rejected), so env_vars has carried the same shape since 2.9.0. The difference is likelihood: an env-var write needs the model to have decided on a value, whereas app config round-trips are a normal workflow and the masked fields are ones a model would echo back without thinking about them.

Cheapest fix that closes it for all three resources: in updateApplication/updateDatabase/updateService, reject (don't silently drop) any SENSITIVE_RESOURCE_FIELDS key whose value is exactly MASKED_VALUE, with an error naming the field and telling the caller to omit it or pass the real value. That is a few lines in one place, and it turns a silent config wipe into a message the model can act on.

2. The masking stops at the top level — what else does the detail payload inline? (medium)

maskResourceItemFull only touches named top-level fields, and the projection only knows about server and destination.server. You found live that /applications/{uuid} inlines the full destination.server relation; the open question is what else it inlines. Two candidates that would still ship in the clear:

• source — for a GitHub-App-sourced app this is a GithubApp row, which carries client_secret, webhook_secret and private_key. The codebase already treats those as sensitive (toGitHubAppSummary drops all three).
• private_key — the deploy key relation. maskPrivateKey exists precisely because that PEM is served decrypted pre-4.2 (private_keys returns full key material on pre-4.2 instances #327).

Worth one live Object.keys(app) dump against 4.1.2 to settle it, then either extend the projection or leave a comment recording that those relations aren't serialized on this endpoint. Given this PR exists because the nested server was a surprise, I would not assume the other relations are absent.

3. Docs not updated in the same PR (low, but it's a CLAUDE.md rule)

CLAUDE.md's Documentation Standards ask for CHANGELOG + README in the PR that changes behaviour. Neither is here:

• CHANGELOG.md — 2.19.2 covers private_keys returns full key material on pre-4.2 instances #327/get_database / get_service raw passthrough includes plaintext credentials #328 under ### Security; get_application raw passthrough leaks webhook secrets and the full server row #332 needs its own entry (or to join that section if it ships in the same release).
• README.md:113-119 — the "Secure by default" bullet list names get_database / get_service and deployment get but not get_application. site/src/pages/index.astro carries parallel copy.

4. Stale jsdoc on the shared helper (low)

src/lib/coolify-client.ts:709-718 still reads "Sanitize a database/service detail payload (#328)". It is now the application path too, and the reveal rationale differs slightly (webhook secrets and compose bodies rather than "wire an app to this database"). Same for the projectNestedServers comment at :686-693, which cites only pre-4.2 /databases/{uuid} and /services/{uuid}.

5. Application.destination doesn't declare server (low)

src/types/coolify.ts:249-255 declares destination as { id, uuid, name, server_id } — no server. The projection you now depend on is entirely cast-driven (out.destination as Record<string, unknown>), so nothing in the type system records that the nested server row exists or that it gets projected on the way out. Adding server?: ServerSummary, with the same "verified live on the detail endpoint" note the surrounding fields carry, would make the next reader's model of this payload correct.

6. listDatabases / listServices full paths are now asymmetric (low)

listApplications non-summary sanitizes as of this PR; listDatabases (:1449-1456) and listServices (:1562-1569) still return raw rows. Nothing leaks today — every model-facing call passes summary: true, and the internal consumers only read uuid/name/status/environment_id — but the asymmetry is exactly the kind that becomes a leak the day someone exposes a full list. Either mirror the treatment or add a one-liner saying why the application path needed it and these don't.

7. Test coverage gaps (low)

• No mcp-server.test.ts assertion that get_application forwards reveal (and defaults it to undefined). There's precedent at src/__tests__/mcp-server.test.ts:581-607 for the env-var reveal wiring. Cheap, and it pins the default-masked posture at the tool boundary rather than only at the client.
• The claim this PR rests on is about the live payload shape, and the fixture only models what you already knew to look for. A smoke test next to the Переменные окружения базы данных по умолчанию возвращаются в открытом виде #276 one (src/__tests__/integration/smoke.integration.test.ts:43) asserting get_application output contains no sentinel_token / BEGIN OPENSSH PRIVATE KEY / client_secret substrings would both regression-guard this and answer finding feat: make package executable for Claude Desktop #2 automatically.

Performance / security notes

No concerns. The projection is a meaningful token win on a payload that embedded the whole proxy compose file, and the masking cost is one shallow spread per row on paths that already parse the full JSON. toEqual on the projected server is the right matcher here — toStrictEqual would fail on is_reachable: undefined.

None of the above blocks the security fix, which is correct as far as it goes. #1 and #2 are the two I'd want resolved before calling this done — #1 because the fix can silently destroy config, #2 because a partial mask reads like a complete one.