Skip to content

Commit 257ab85

Browse files
TheLastCicadaTheLastCicada
committed
refactor(API): disable /diagnostics in read-only mode instead of stripping fields
Return 403 when READ_ONLY is set rather than serving a reduced response. Removes buildReadOnlyResponse and the readOnly parameter from getDiagnosticsResponse since the middleware now gates access.
1 parent bcde8d9 commit 257ab85

4 files changed

Lines changed: 25 additions & 164 deletions

File tree

‎src/middleware.js‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,16 +525,20 @@ app.get('/health', (req, res) => {
525525
// that single enforcement point rather than duplicating the constant-time
526526
// check here.
527527
//
528-
// Read-only: when V1 or V2 READ_ONLY is set we strip sensitive fields
529-
// (balances, transaction details, peer details, subscription IDs, home org
530-
// IDs) the same way wallet-health.js does for /v{1,2}/health/wallet.
528+
// Disabled in read-only mode: the diagnostics payload exposes system details
529+
// (paths, wallet balances, peer IPs, subscription IDs) that should not be
530+
// served on unauthenticated public-observer nodes.
531531
app.get('/diagnostics', async (req, res) => {
532532
try {
533533
const configV1 = getConfig();
534534
const configV2 = getConfigV2();
535-
const readOnly = !!(configV2.READ_ONLY || configV1.READ_ONLY);
535+
if (configV2.READ_ONLY || configV1.READ_ONLY) {
536+
return res.status(403).json({
537+
error: 'The /diagnostics endpoint is not available on read-only nodes',
538+
});
539+
}
536540
const { getDiagnosticsResponse } = await import('./routes/diagnostics.js');
537-
const result = await getDiagnosticsResponse({ readOnly });
541+
const result = await getDiagnosticsResponse();
538542
return res.status(200).json(result);
539543
} catch (error) {
540544
logger.error(`[diagnostics]: unexpected error building response: ${error.message}`);

‎src/routes/diagnostics.js‎

Lines changed: 3 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@
77
* when something is broken, so every external call is fenced behind
88
* `Promise.allSettled` with a per-call timeout. The route never throws.
99
*
10-
* In read-only mode the same shape is returned but with sensitive fields
11-
* (balances, transaction details, peer host details, subscription IDs,
12-
* home org IDs) stripped, matching the convention from wallet-health.js.
10+
* The endpoint is disabled entirely in read-only mode (handled by
11+
* middleware returning 403), so there is no reduced-information path.
1312
*/
1413

1514
import _ from 'lodash';
@@ -215,32 +214,14 @@ const buildTrustedPeerView = (connectionsResult, chiaConfigResult) => {
215214
* Heavy module dependencies (database models, datalayer RPC clients) are
216215
* loaded dynamically so this file is safe to import from contexts where
217216
* those modules aren't ready yet (tests, early startup).
218-
*
219-
* In read-only mode we deliberately skip the expensive wallet/datalayer/
220-
* full-node RPC fan-out -- read-only ("public observer") deployments
221-
* should not be making per-request authenticated wallet RPC calls on
222-
* unauthenticated public hits. This matches the precedent in
223-
* src/routes/wallet-health.js, which also short-circuits before fetch.
224217
*/
225-
export const getDiagnosticsResponse = async ({ readOnly = false } = {}) => {
218+
export const getDiagnosticsResponse = async () => {
226219
const timestamp = new Date().toISOString();
227220

228221
const configV1 = getConfig();
229222
const configV2 = getConfigV2();
230223
const appConfig = configV1.APP || {};
231224

232-
// Short-circuit BEFORE pulling in the heavy wallet/datalayer/model
233-
// dependencies. The read-only path only needs system-info / process-scan /
234-
// chia-tools probes plus the synchronously-available config, so importing
235-
// wallet.js or models/v2/index.js here would (a) defeat the "no
236-
// authenticated RPC on unauthenticated public hits" property and (b)
237-
// make /diagnostics itself fail whenever the DB/wallet modules can't
238-
// initialize -- which is precisely the failure mode this endpoint exists
239-
// to diagnose.
240-
if (readOnly) {
241-
return buildReadOnlyResponse({ timestamp, configV1, configV2, appConfig });
242-
}
243-
244225
const wallet = (await import('../datalayer/wallet.js')).default;
245226
const fullNodeRpc = (await import('../datalayer/fullNodeRpc.js')).default;
246227
const persistance = await import('../datalayer/persistance.js');
@@ -475,7 +456,6 @@ export const getDiagnosticsResponse = async ({ readOnly = false } = {}) => {
475456
// ---- Full response ------------------------------------------------------
476457
const fullResponse = {
477458
timestamp,
478-
readOnly: false,
479459
cadt: cadtSection,
480460
network: {
481461
chia: actualNetwork,
@@ -497,68 +477,6 @@ export const getDiagnosticsResponse = async ({ readOnly = false } = {}) => {
497477
return fullResponse;
498478
};
499479

500-
/**
501-
* Read-only diagnostics: only cheap, public-safe data. We short-circuit
502-
* BEFORE the wallet/datalayer/full-node RPC fan-out so a read-only
503-
* deployment doesn't make per-request authenticated wallet RPC calls on
504-
* unauthenticated public hits.
505-
*/
506-
const buildReadOnlyResponse = async ({ timestamp, configV1, configV2, appConfig }) => {
507-
const enableV1 = configV1?.ENABLE !== false;
508-
const enableV2 = configV2?.ENABLE !== false;
509-
const chiaRoot = getChiaRoot();
510-
511-
const [systemInfoRes, chiaProcessesRes, chiaToolsRes] = await Promise.all([
512-
settle('getSystemInfo', () => getSystemInfo(), DEFAULT_TIMEOUT_MS),
513-
settle('scanChiaProcesses', () => scanChiaProcesses(), DEFAULT_TIMEOUT_MS),
514-
settle('probeChiaTools', () => probeChiaTools(), DEFAULT_TIMEOUT_MS),
515-
]);
516-
517-
const processesValue = chiaProcessesRes.ok
518-
? chiaProcessesRes.value
519-
: { matches: [], installPaths: [], multipleVersionsDetected: false, error: chiaProcessesRes.error };
520-
const chiaToolsSection = chiaToolsRes.ok
521-
? chiaToolsRes.value
522-
: { installed: false, version: null, error: chiaToolsRes.error };
523-
const systemSection = systemInfoRes.ok ? systemInfoRes.value : { error: systemInfoRes.error };
524-
525-
return {
526-
timestamp,
527-
readOnly: true,
528-
message: 'Operational details are not available on read-only nodes',
529-
cadt: {
530-
version: packageJson.version,
531-
configDir: `${chiaRoot}/cadt`,
532-
configFile: `${chiaRoot}/cadt/config.yaml`,
533-
datalayerUrl: appConfig.DATALAYER_URL || null,
534-
datalayerFileServerUrl: appConfig.DATALAYER_FILE_SERVER_URL || null,
535-
useSimulator: appConfig.USE_SIMULATOR === true,
536-
v1: {
537-
enabled: enableV1,
538-
readOnly: configV1.READ_ONLY === true,
539-
isGovernanceBody: configV1.IS_GOVERNANCE_BODY === true,
540-
apiKeyConfigured: !!(configV1.CADT_API_KEY && configV1.CADT_API_KEY !== ''),
541-
governanceBodyId: configV1.GOVERNANCE?.GOVERNANCE_BODY_ID || null,
542-
},
543-
v2: {
544-
enabled: enableV2,
545-
readOnly: configV2.READ_ONLY === true,
546-
isGovernanceBody: configV2.IS_GOVERNANCE_BODY === true,
547-
apiKeyConfigured: !!(configV2.CADT_API_KEY && configV2.CADT_API_KEY !== ''),
548-
governanceBodyId: configV2.GOVERNANCE?.GOVERNANCE_BODY_ID || null,
549-
},
550-
},
551-
network: { cadt: appConfig.CHIA_NETWORK || null },
552-
chia: {
553-
chiaTools: { installed: chiaToolsSection.installed, version: chiaToolsSection.version },
554-
runningProcesses: {
555-
multipleVersionsDetected: processesValue.multipleVersionsDetected,
556-
},
557-
},
558-
system: systemSection,
559-
};
560-
};
561-
562480
export const __test = {
563481
settle,
564482
collectSubscriptions,

‎tests/v2/integration/diagnostics.spec.js‎

Lines changed: 13 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ describe('/diagnostics endpoint', function () {
1818
it('responds 200 with the documented top-level shape', async function () {
1919
const response = await supertest(app).get('/diagnostics').expect(200);
2020
expect(response.body).to.have.property('timestamp').that.is.a('string');
21-
expect(response.body).to.have.property('readOnly');
2221
expect(response.body).to.have.property('cadt').that.is.an('object');
2322
expect(response.body).to.have.property('network').that.is.an('object');
2423
expect(response.body).to.have.property('chia').that.is.an('object');
@@ -164,89 +163,30 @@ describe('/diagnostics endpoint', function () {
164163
});
165164
});
166165

167-
describe('read-only stripping', function () {
168-
// The exact contract is documented in src/routes/diagnostics.js and
169-
// mirrors the wallet-health.js convention: when READ_ONLY is true the
170-
// response retains only non-sensitive metadata. Balances, transaction
171-
// details, peer details, subscription IDs, and home-org IDs MUST be
172-
// absent. We also intentionally short-circuit BEFORE the wallet/datalayer
173-
// RPC fan-out, so the entire `chia.wallet` and `chia.datalayer` sections
174-
// are omitted in read-only mode.
175-
it('omits operational details when READ_ONLY is enabled', async function () {
176-
await withConfigOverride(async () => {
177-
const response = await supertest(app).get('/diagnostics').expect(200);
178-
expect(response.body).to.have.property('readOnly', true);
179-
expect(response.body).to.have.property('message').that.is.a('string');
180-
181-
expect(response.body.chia).to.not.have.property('wallet');
182-
expect(response.body.chia).to.not.have.property('datalayer');
183-
expect(response.body.chia).to.not.have.property('fullNode');
184-
expect(response.body.chia).to.not.have.property('services');
185-
186-
expect(response.body.cadt.v1).to.not.have.property('homeOrgId');
187-
expect(response.body.cadt.v2).to.not.have.property('homeOrgId');
188-
189-
expect(response.body.cadt).to.have.property('version');
190-
expect(response.body.network).to.have.property('cadt');
191-
expect(response.body.chia).to.have.property('chiaTools');
192-
expect(response.body).to.have.property('system');
193-
}, { APP: { READ_ONLY: true } });
194-
});
195-
196-
it('keeps full detail by default (READ_ONLY off)', async function () {
197-
const response = await supertest(app).get('/diagnostics').expect(200);
198-
expect(response.body.readOnly).to.equal(false);
199-
expect(response.body.chia.wallet).to.have.property('pendingTransactions');
200-
expect(response.body.chia.wallet).to.have.property('trustedFullNodePeers');
201-
expect(response.body.chia.datalayer).to.have.property('subscriptions');
202-
});
203-
204-
// The four scenarios below exhaustively cover the (READ_ONLY × API key
205-
// configured × key provided) matrix for READ_ONLY=true. They confirm both
206-
// (a) a public observer node with no API key gets the reduced response
207-
// without authentication, and (b) when an API key IS configured the
208-
// endpoint still enforces it, returning the reduced response only to
209-
// authenticated callers.
210-
it('public observer node (READ_ONLY=true, no API key): returns 200 with reduced fields without auth', async function () {
211-
await withConfigOverride(async () => {
212-
const response = await supertest(app).get('/diagnostics');
213-
expect(response.status).to.equal(200);
214-
expect(response.body.readOnly).to.equal(true);
215-
expect(response.body.chia).to.not.have.property('wallet');
216-
expect(response.body.chia).to.not.have.property('datalayer');
217-
expect(response.body.chia).to.not.have.property('fullNode');
218-
expect(response.body.cadt).to.have.property('version');
219-
expect(response.body.network).to.have.property('cadt');
220-
expect(response.body).to.have.property('system');
221-
}, { APP: { READ_ONLY: true, CADT_API_KEY: '' } });
222-
});
223-
224-
it('READ_ONLY=true + API key configured + no key provided: rejects with 403', async function () {
166+
describe('read-only mode', function () {
167+
it('returns 403 when READ_ONLY is enabled', async function () {
225168
await withConfigOverride(async () => {
226169
const response = await supertest(app).get('/diagnostics');
227170
expect(response.status).to.equal(403);
228-
}, { APP: { READ_ONLY: true, CADT_API_KEY: 'protected-observer-key' } });
171+
expect(response.body).to.have.property('error').that.is.a('string');
172+
}, { APP: { READ_ONLY: true } });
229173
});
230174

231-
it('READ_ONLY=true + API key configured + correct key: returns 200 with reduced fields', async function () {
175+
it('returns 403 regardless of API key when READ_ONLY is enabled', async function () {
232176
await withConfigOverride(async () => {
233177
const response = await supertest(app)
234178
.get('/diagnostics')
235179
.set('x-api-key', 'protected-observer-key');
236-
expect(response.status).to.equal(200);
237-
expect(response.body.readOnly).to.equal(true);
238-
expect(response.body.chia).to.not.have.property('wallet');
239-
expect(response.body.chia).to.not.have.property('datalayer');
180+
expect(response.status).to.equal(403);
181+
expect(response.body).to.have.property('error').that.is.a('string');
240182
}, { APP: { READ_ONLY: true, CADT_API_KEY: 'protected-observer-key' } });
241183
});
242184

243-
it('READ_ONLY=true + API key configured + wrong key: rejects with 403', async function () {
244-
await withConfigOverride(async () => {
245-
const response = await supertest(app)
246-
.get('/diagnostics')
247-
.set('x-api-key', 'X'.repeat('protected-observer-key'.length));
248-
expect(response.status).to.equal(403);
249-
}, { APP: { READ_ONLY: true, CADT_API_KEY: 'protected-observer-key' } });
185+
it('returns full detail by default (READ_ONLY off)', async function () {
186+
const response = await supertest(app).get('/diagnostics').expect(200);
187+
expect(response.body.chia.wallet).to.have.property('pendingTransactions');
188+
expect(response.body.chia.wallet).to.have.property('trustedFullNodePeers');
189+
expect(response.body.chia.datalayer).to.have.property('subscriptions');
250190
});
251191
});
252192

@@ -262,7 +202,7 @@ describe('/diagnostics endpoint', function () {
262202
it('always returns 200 with the documented top-level keys', async function () {
263203
const response = await supertest(app).get('/diagnostics').expect(200);
264204
expect(response.body).to.have.all.keys(
265-
'timestamp', 'readOnly', 'cadt', 'network', 'chia', 'system',
205+
'timestamp', 'cadt', 'network', 'chia', 'system',
266206
);
267207
expect(response.body.system).to.have.property('cpu');
268208
expect(response.body.system).to.have.property('memory');

‎tests/v2/live-api/wallet-health.live.spec.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ describe('Wallet Health - Live', function () {
9696
expect(body.timestamp, 'timestamp must be ISO-8601').to.match(
9797
/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/,
9898
);
99-
expect(body.readOnly).to.be.a('boolean');
10099
expect(body.cadt).to.be.an('object');
101100
expect(body.network).to.be.an('object');
102101
expect(body.chia).to.be.an('object');

0 commit comments

Comments
 (0)