From fedbe816908bbda877efe2e87106c2b93c442814 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Wed, 5 Aug 2026 18:41:50 -0400 Subject: [PATCH] fix: only catalog hosts are jump targets (v1.19.0) isManagedHost() treated a missing metadata.managed flag as permission, so any host the SSO merely discovered -- an unpromoted Proxmox guest, a UniFi client -- was offered in the TUI picker and accepted by the username grammar. Replaced with isCatalogHost(), mirroring the rule the SSO Directory's own listing applies: a resource carrying discovery_sources but never promoted is excluded; hand-created hosts and promoted ones are included; an explicit managed:false is always excluded. The two copies of this rule have now drifted apart once. If a third consumer needs it, hoist it into @simpleworkjs/directory-schema rather than copying again. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 +++++ docs/connecting.md | 14 +++++++++++--- nodejs/package-lock.json | 4 ++-- nodejs/package.json | 2 +- nodejs/test/unit/access.test.js | 19 +++++++++++++++++++ nodejs/utils/access.js | 29 ++++++++++++++++++++--------- 6 files changed, 58 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcd14bc..ee0ec9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v1.19.0 +- fix: **only catalog hosts are jump targets.** `isManagedHost` treated a missing `metadata.managed` flag as permission, so any host the SSO merely *discovered* — an unpromoted Proxmox guest, a UniFi client — was offered in the TUI picker and accepted by the username grammar. The filter is now `isCatalogHost`, mirroring the SSO Directory's own rule: a resource carrying `discovery_sources` but never promoted is excluded, while hand-created hosts (no `discovery_sources`) and promoted ones (`managed: true`) are included, and an explicit `managed: false` is always excluded. +- test: regression coverage for all five cases (hand-made, discovered-unpromoted, discovered-promoted, `manual` source, explicitly unmanaged). +- docs: `docs/connecting.md` states that discovery results are not jump targets until promoted into the catalog. + ## v1.18.0 - feat: Add SSO-style error page (404/500) for browser navigation instead of a bare text response - feat: navbar — username no longer underlined; only the active link is bold + underlined diff --git a/docs/connecting.md b/docs/connecting.md index 966ba87..4e4b9ab 100644 --- a/docs/connecting.md +++ b/docs/connecting.md @@ -66,14 +66,22 @@ directory access allows — it doubles as "what can I reach from here?" ## What you can reach The set of hosts is computed per login: your LDAP group memberships intersected -with the SSO directory's hosts (via the `host__access` groups the -directory auto-creates for each machine). To get access to a new host, an admin -adds you to that host's access group in the SSO — nothing on the jump host +with the SSO directory's **catalog** hosts (via the `host__access` groups +the directory auto-creates for each machine). To get access to a new host, an +admin adds you to that host's access group in the SSO — nothing on the jump host changes. Targets that don't resolve to a host you're allowed to reach are refused (and audited). Raw IPs that aren't a known directory host are denied by default. +**Only catalog hosts are jump targets.** A machine that the SSO merely +*discovered* — a Proxmox guest, a UniFi client — is not a jump target until an +admin promotes it into the directory catalog. The jump host applies the same +rule the SSO's own Directory listing does: a resource carrying +`discovery_sources` but never promoted is excluded, while hand-created hosts and +promoted ones are included. Previously the filter treated a missing `managed` +flag as permission, so unpromoted discovery results showed up in the picker. + > On a [standalone](architecture.html#standalone-mode) jump host (no LDAP/SSO), > every registered host is reachable by every registered user — there's no > group-based restriction to ask an admin about. diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index c598836..2f7cf78 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "t42-jump-host", - "version": "1.18.0", + "version": "1.19.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "t42-jump-host", - "version": "1.18.0", + "version": "1.19.0", "license": "MIT", "dependencies": { "@fortawesome/fontawesome-free": "^7.3.0", diff --git a/nodejs/package.json b/nodejs/package.json index b9ccba0..5ab0a16 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "t42-jump-host", - "version": "1.18.0", + "version": "1.19.0", "description": "SSH jump host for the theta42 stack — LDAP-authenticated, directory-driven host bridging with audit and metrics", "author": [ { diff --git a/nodejs/test/unit/access.test.js b/nodejs/test/unit/access.test.js index c9aebe0..1d883d5 100644 --- a/nodejs/test/unit/access.test.js +++ b/nodejs/test/unit/access.test.js @@ -70,6 +70,25 @@ test('allHosts fetches the whole host inventory with no group filter', async () assert.deepStrictEqual(hosts.map((h) => h.id).sort(), ['1', '2']); }); +// Only catalog content is a jump target. Discovery writes `discovery_sources`; +// promoting to the catalog sets `managed: true`. An unpromoted Proxmox VM was +// reaching the picker because the filter defaulted `managed`-less hosts to true. +test('drops auto-discovered hosts that were never promoted', async () => { + clearCache(); + const user = { uid: 'frank', dn: 'd' }; + const fetchImpl = stubFetch({ + frank: [ + { id: '1', kind: 'host', slug: 'host_web01' }, // hand-made: no discovery_sources + { id: '2', kind: 'host', slug: 'vm-101', metadata: { discovery_sources: ['proxmox'] } }, // discovered, unpromoted + { id: '3', kind: 'host', slug: 'vm-102', metadata: { discovery_sources: ['proxmox'], managed: true } }, // promoted + { id: '4', kind: 'host', slug: 'host_db', metadata: { discovery_sources: ['manual'] } }, // manual source counts as catalog + { id: '5', kind: 'host', slug: 'host_off', metadata: { managed: false } }, // explicitly out + ], + }); + const hosts = await accessibleHosts(user, { fetchImpl }); + assert.deepStrictEqual(hosts.map((h) => h.id).sort(), ['1', '3', '4']); +}); + test('a bare-array response (envelope drift) returns empty list', async () => { clearCache(); const user = { uid: 'dave', dn: 'd' }; diff --git a/nodejs/utils/access.js b/nodejs/utils/access.js index 451c56e..4a442bc 100644 --- a/nodejs/utils/access.js +++ b/nodejs/utils/access.js @@ -46,19 +46,30 @@ if (conf.standalone && conf.standalone.enabled) { // Every host in the inventory, unfiltered — for admins (the web UI's own // account is already gated by requireAdmin before this is ever called). - function isManagedHost(r) { + // + // "In the catalog" is the same predicate the SSO's own Directory listing + // applies (sso-manager-node routes/api_directory_admin.js GET /resources): + // a resource that was auto-discovered and never promoted is NOT catalog + // content and must never be offered as a jump target. Discovery writes + // `metadata.discovery_sources`; promoting sets `metadata.managed = true`. + // Hosts created by hand carry no discovery_sources at all and stay in. + // + // The two copies of this rule have already drifted apart once (unpromoted + // Proxmox VMs showing up in the picker); if a third consumer needs it, + // hoist it into @simpleworkjs/directory-schema rather than copying again. + function isCatalogHost(r) { if (!r || r.kind !== 'host') return false; - // If managed attribute is present, require it to be true/truthy - if (r.metadata && r.metadata.managed !== undefined) { - return r.metadata.managed === true || r.metadata.managed === 'true'; - } - // Default to true for manually created hosts that lack explicit managed metadata - return true; + const meta = r.metadata || {}; + if (meta.managed === true || meta.managed === 'true') return true; + if (meta.managed === false || meta.managed === 'false') return false; + const sources = meta.discovery_sources || []; + const autoDiscovered = sources.length > 0 && !sources.includes('manual'); + return !autoDiscovered; } async function allHosts({ fetchImpl = fetch } = {}) { const resources = await directoryClient({ fetchImpl }).getResourcesByGroup(undefined, { kind: 'host' }); - return resources.filter(isManagedHost); + return resources.filter(isCatalogHost); } async function accessibleHosts(user, { fetchImpl = fetch } = {}) { @@ -72,7 +83,7 @@ if (conf.standalone && conf.standalone.enabled) { console.error(`[access] ${error.message}`); } - const hosts = resources.filter(isManagedHost); + const hosts = resources.filter(isCatalogHost); cache.set(user.uid, { at: Date.now(), hosts }); return hosts; }