Skip to content

Commit 1dd983d

Browse files
committed
feat(core): S-6 plugin:doctor + plugin_extension_identifier guard + threat model (closes Lote S)
The last slice of Lote S. Adds operator-facing diagnostics, the fourth plugin-surface Isthmus guard, and the published trust-boundary threat model. - guard.plugin_extension_identifier: the branded-name minters (sdk/brands.ts) now validate with the non-emitting isSafeIdentifier predicate and emit their OWN plugin-surface guard on a reject (distinct from the tenant-DDL guard.tenant_identifier), so a hostile plugin identifier reads apart from a bad tenant id. Same accept/reject set. Registry entry + matrix recipe + fuzz-spec assertion updated. - plugin:doctor command + pure PluginDoctorService: diagnoses the deployed platform posture from the discovered manifests + trust/firewall config — Satellite ABI drift (error/warn), native-addon sandbox risk, dead TRUSTED_SATELLITES entries, a missing read-only firewall while untrusted plugins are installed, and a disclosure of every declared (consent-gated) permission. It does NOT introspect specs (that is the check-plugin-permissions CI guard). Full command ceremony (commands.json, index, commands.md); the service is unit-tested per check. - Threat model: docs/guides/security.md gains the five-layer plugin trust-boundary matrix (S1..S5 + the in-process-sandbox non-goal, each verdict real/friction) plus hardening-checklist items; stability.md gets a plugin-platform row (Experimental); .github/SECURITY.md states the sandbox-escape non-goal. - Coverage floors ratcheted lines/statements 64 -> 65. Verified via a 4-lens adversarial review (bypass/tests/ceremony/honesty); 3 low findings folded in (severity assertions for 3 doctor checks, disclose ALL declared permission kinds not a subset, corrected a stale plugin_env comment). Gates green: core unit 1575, tsc 0 all workspaces, check 43/43, api-report 3/3, lint 0, Isthmus audit-coverage 100% (23 registered guards), coverage green at the new floors. Lote S (S-0..S-6) is complete. Merge order stays A -> S -> B -> C.
1 parent 42450a0 commit 1dd983d

15 files changed

Lines changed: 532 additions & 19 deletions

File tree

.github/SECURITY.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,9 @@ Reports against the core isolation guarantees (cross-tenant read or write
3131
leakage, resolver hijack, fail-open under dependency outage) are highest
3232
priority. See the [stability matrix](https://arcoders.github.io/Adonisjs-lasagna-saas-tenancy/reference/stability)
3333
for the per-feature labels.
34+
35+
An installed plugin/satellite runs in-process with full privilege by design, so an
36+
**in-process plugin sandbox escape is not a goal** — the in-process trust controls are
37+
friction, and the hard boundary for an untrusted plugin's write is the read-only
38+
Postgres role (S3). The [plugin platform trust boundary](https://arcoders.github.io/Adonisjs-lasagna-saas-tenancy/guides/security#plugin-platform-trust-boundary)
39+
in the security guide states the five-layer model and what each layer does and does not contain.

docs/guides/security.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,30 @@ PostgreSQL and Redis (no mocks) under contention.
6969
| Cache namespace collision | [`tests/@guarantees/behavior/integration/behavior_cache_for.spec.ts`](https://github.com/Arcoders/Adonisjs-lasagna-saas-tenancy/blob/master/packages/core/tests/@guarantees/behavior/integration/behavior_cache_for.spec.ts) — per-tenant BentoCache namespaces never share keys. |
7070
| Cross-tenant IDOR via tenant-id swap | [`tests/@guarantees/security/integration/security_tenant_guard_authorize.spec.ts`](https://github.com/Arcoders/Adonisjs-lasagna-saas-tenancy/blob/master/packages/core/tests/@guarantees/security/integration/security_tenant_guard_authorize.spec.ts): with `authorizeTenantAccess` wired, a caller whose principal belongs to tenant A gets 403 when resolving to tenant B. |
7171

72+
## Plugin platform trust boundary
73+
74+
A satellite installed as a provider runs **in-process with full privilege**. The
75+
plugin platform does not pretend otherwise: an installed plugin can `import` any
76+
module and reach any singleton, so nothing implemented in JavaScript is a sandbox.
77+
The controls below raise the cost of a careless or opportunistic plugin and give an
78+
operator an honest, consented install, but only two of them are hard boundaries, and
79+
both are enforced outside the JavaScript VM.
80+
81+
| Layer | Control | Enforced by | Verdict |
82+
|---|---|---|---|
83+
| **S1 — Install consent** | The plugin declares its sensitive `permissions`; `configure` shows them and refuses a non-interactive install without `--accept-permissions`. | A human at install time | Real gate (human) |
84+
| **S2 — Supply chain** | Native-addon install gate (`--allow-native`), plus `lasagna:health-check` (`npm audit` over the tree, install-script and native-addon flags). | Install / CI | Real (raises the bar; not runtime) |
85+
| **S3 — Read-only DB role** | Untrusted plugin code is routed to a per-tenant connection authenticated as a `NOSUPERUSER NOBYPASSRLS` role with `default_transaction_read_only = on`. | PostgreSQL | **Hard boundary** (Postgres denies the write) |
86+
| **S4 — Out-of-process worker** | Background plugin code (scheduler / data-change, once Lote B/C land) runs in a dedicated worker under the Node Permission Model (`--permission`, fs/child_process narrowed). A native-addon plugin fails boot without `--allow-addons`. | Node runtime + orchestrator | Real when configured (native addons opt out — treat as trusted) |
87+
| **S5 — In-process friction** | Trusted-list proxies over the tenant repository and the db handle; an allowlist over sensitive capability provide/consume. | JavaScript | **Friction, not a boundary** (a direct import reaches around it) |
88+
| In-process sandbox escape ||| **Non-goal** — an installed plugin has full in-process reach by design |
89+
90+
The one control that actually denies an untrusted plugin's write is **S3, the
91+
read-only Postgres role**. Configure it (`plugins.readOnly`) and put untrusted
92+
third-party plugins outside `TRUSTED_SATELLITES`. `plugin:doctor` reports the standing
93+
posture: ABI drift, native-addon risk, a stale trust allowlist, and a missing read-only
94+
firewall. See the [plugins guide](/guides/plugins) for authoring against these controls.
95+
7296
## Hardening checklist for production
7397

7498
Before going live, work through this list; every item is a host
@@ -87,6 +111,7 @@ responsibility (the package gives you the primitives).
87111
- [ ] OIDC `client_secret`, encryption keys, and S3 credentials live in a secrets manager, not `.env` checked into git.
88112
- [ ] `tenant:secrets:reencrypt` has been run **before upgrading** (it brings every stored webhook/SSO secret to the current `APP_KEY` and its per-class context). Reads now fail closed, so any secret left under the legacy shared context or in plaintext stops working until migrated. Run with `OLD_APP_KEY` set for a key rotation, or unset for a context-only migration; it is idempotent and resumable.
89113
- [ ] `tenant:doctor` runs on a cron in production (the [doctor command](/reference/commands#tenant-doctor)) and pages on `error`-level findings.
114+
- [ ] If you install third-party satellites: `plugins.readOnly` is configured (the S3 Postgres read-only role), untrusted plugins are kept off `TRUSTED_SATELLITES`, and `plugin:doctor` runs alongside `tenant:doctor` (see the [plugin platform trust boundary](#plugin-platform-trust-boundary)). Native-addon plugins are treated as fully trusted — they cannot be sandboxed by the worker Permission Model.
90115
- [ ] Health probes wired (`/livez`, `/readyz`, `/healthz`, `/metrics`); see [Health & metrics](/guides/health).
91116

92117
## Reporting vulnerabilities

docs/reference/commands.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,18 @@ advisory, so it doubles as a CI gate.
6161
node ace lasagna:health-check
6262
```
6363

64+
`plugin:doctor` diagnoses the installed plugin/satellite platform posture: Satellite
65+
ABI compatibility of each installed satellite, native-addon sandbox risk, a stale or
66+
typo'd `TRUSTED_SATELLITES` allowlist, whether the read-only firewall (`plugins.readOnly`)
67+
is configured while untrusted plugins are installed, and which plugins hold declared
68+
(consent-gated) permissions. It exits non-zero on any error, so it doubles as a CI gate. It does not
69+
re-check manifest↔spec coherence — that is the `check-plugin-permissions` build guard.
70+
71+
```bash
72+
node ace plugin:doctor
73+
node ace plugin:doctor --json
74+
```
75+
6476
## Doctor
6577

6678
`tenant:doctor` is the operational health command. Nine built-in

docs/reference/stability.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ The isolation substrate. Everything here is **release candidate** unless noted.
9898
| Health probes (`/livez`, `/readyz`, `/healthz`) | Release candidate | |
9999
| Doctor (base checks) | Release candidate | |
100100
| Plans and quotas (`enforceQuota`) | Experimental | Opt-in feature, not part of the isolation guarantee. |
101+
| Plugin platform (`definePlugin`, request-path seams, capability registry, trust controls) | Experimental | Opt-in extension surface for third-party satellites. The in-process trust controls are friction, not a sandbox; the read-only role is the boundary. See [plugins](/guides/plugins) and the [security guide](/guides/security). |
101102
| Read-replica routing | Experimental | No automatic failover by design; use the retry-on-primary pattern in [read replicas](/guides/read-replicas). |
102103
| Audit logs | Experimental | Satellite (in core). |
103104
| Webhooks | Experimental | Satellite (in core). |

packages/core/.c8rc.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
"report-dir": "./coverage",
1616
"clean": true,
1717
"check-coverage": true,
18-
"lines": 64,
18+
"lines": 65,
1919
"branches": 85,
2020
"functions": 75,
21-
"statements": 64
21+
"statements": 65
2222
}

packages/core/src/commands/commands.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,19 @@
532532
"args": [],
533533
"options": { "startApp": false },
534534
"filePath": "health_check.js"
535+
},
536+
{
537+
"commandName": "plugin:doctor",
538+
"description": "Diagnose the installed plugin/satellite platform posture — ABI compatibility, native-addon sandbox risk, the TRUSTED_SATELLITES allowlist, and the read-only firewall",
539+
"help": "",
540+
"namespace": "plugin",
541+
"aliases": [],
542+
"flags": [
543+
{ "name": "json", "flagName": "json", "required": false, "type": "boolean", "description": "Emit a JSON report on stdout instead of the table" }
544+
],
545+
"args": [],
546+
"options": { "startApp": true },
547+
"filePath": "plugin_doctor.js"
535548
}
536549
],
537550
"version": 1

packages/core/src/commands/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@ export { default as TenantGdprAnonymize } from './tenant_gdpr_anonymize.js'
3434
export { default as TenantComplianceReport } from './tenant_compliance_report.js'
3535
export { default as TenantVectorProvision } from './tenant_vector_provision.js'
3636
export { default as HealthCheck } from './health_check.js'
37+
export { default as PluginDoctor } from './plugin_doctor.js'
3738
// The `tenant:billing:*` commands moved to `@adonisjs-lasagna/billing`.
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { BaseCommand, flags } from '@adonisjs/core/ace'
2+
import type { CommandOptions } from '@adonisjs/core/types/ace'
3+
import { getConfig } from '../config.js'
4+
import { discoverSatellites } from '../sdk/configure_kit.js'
5+
import { trustedSatellites } from '../sdk/plugin_env.js'
6+
import PluginDoctorService from '../services/plugin_doctor_service.js'
7+
import type { DiagnosisSeverity } from '../services/doctor/types.js'
8+
9+
const SEVERITY_COLOR: Record<DiagnosisSeverity, 'cyan' | 'yellow' | 'red'> = {
10+
info: 'cyan',
11+
warn: 'yellow',
12+
error: 'red',
13+
}
14+
15+
/**
16+
* `plugin:doctor` — diagnose the installed plugin/satellite PLATFORM posture (S6).
17+
* It reads the discovered satellite manifests plus the runtime trust and firewall
18+
* configuration and reports ABI drift, native-addon sandbox risk, a stale
19+
* `TRUSTED_SATELLITES` allowlist, a missing read-only firewall, and which plugins
20+
* hold declared (consent-gated) permissions. The checking logic is the pure `PluginDoctorService`;
21+
* this command is the thin shell that gathers the input and renders the report.
22+
*
23+
* It does NOT re-check manifest↔spec coherence — that is the `check-plugin-permissions`
24+
* CI guard's job (the specs are not available at runtime). Exit code 1 on any error.
25+
*/
26+
export default class PluginDoctor extends BaseCommand {
27+
static readonly commandName = 'plugin:doctor'
28+
static readonly description =
29+
'Diagnose the installed plugin/satellite platform posture — ABI compatibility, ' +
30+
'native-addon sandbox risk, the TRUSTED_SATELLITES allowlist, and the read-only firewall'
31+
static readonly options: CommandOptions = { startApp: true }
32+
33+
@flags.boolean({
34+
flagName: 'json',
35+
default: false,
36+
description: 'Emit a JSON report on stdout instead of the table',
37+
})
38+
declare json: boolean
39+
40+
async run() {
41+
const root = this.app.makePath()
42+
const discovered = await discoverSatellites(root, (m) => this.logger.warning(m))
43+
const config = getConfig()
44+
45+
const report = new PluginDoctorService().run({
46+
satellites: discovered.map((s) => ({
47+
packageName: s.packageName,
48+
name: s.manifest.name,
49+
satelliteApi: s.manifest.satelliteApi,
50+
permissions: s.manifest.permissions,
51+
nativeAddons: s.manifest.nativeAddons,
52+
})),
53+
trusted: trustedSatellites(),
54+
readOnlyConfigured: config.plugins?.readOnly !== undefined,
55+
})
56+
57+
if (this.json) {
58+
this.logger.log(JSON.stringify(report, null, 2))
59+
this.exitCode = report.totals.error > 0 ? 1 : 0
60+
return
61+
}
62+
63+
this.logger.info(`${discovered.length} installed satellite(s) discovered`)
64+
for (const issue of report.issues) {
65+
const tag = this.colors[SEVERITY_COLOR[issue.severity]](`[${issue.severity.toUpperCase()}]`)
66+
this.logger.log(` ${tag} ${issue.message}`)
67+
}
68+
this.logger.log('')
69+
this.logger.log(
70+
this.colors.bold('Summary ') +
71+
`info: ${report.totals.info} ` +
72+
this.colors.yellow(`warn: ${report.totals.warn}`) +
73+
' ' +
74+
this.colors.red(`error: ${report.totals.error}`)
75+
)
76+
this.exitCode = report.totals.error > 0 ? 1 : 0
77+
}
78+
}

packages/core/src/isthmus/registry.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,22 @@ export const ISTHMUS_REGISTRY = [
123123
reviewed: '2026-07-08',
124124
nextReview: '2027-01-08',
125125
},
126+
{
127+
id: 'guard.plugin_extension_identifier',
128+
pillar: 'guard',
129+
bugClass: 'plugin-identifier-injection',
130+
failMode: 'closed',
131+
phase: 'runtime',
132+
event: 'isthmus:guard:plugin_extension_identifier:rejected',
133+
severity: 'high',
134+
evidence: {
135+
kind: 'inherent-risk',
136+
ref: 'a plugin name, authorizer, capability key, or model name is minted from author input and interpolated into a Redis key, a Symbol, or DDL; a homoglyph or a colon must be rejected at the plugin surface, not folded onto an existing id',
137+
},
138+
guardFile: 'src/sdk/brands.ts',
139+
reviewed: '2026-07-08',
140+
nextReview: '2027-01-08',
141+
},
126142
{
127143
id: 'guard.redirect_host',
128144
pillar: 'guard',

packages/core/src/sdk/brands.ts

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
import { assertSafeIdentifier } from '../services/isolation/identifier.js'
1+
import { isSafeIdentifier } from '../services/isolation/identifier.js'
2+
import { emitIsthmusEvent } from '../isthmus/audit.js'
23

34
/**
45
* Nominal (branded) types for the plugin surface. A value of one of these types
5-
* is PROOF that it already passed {@link assertSafeIdentifier}: the ONLY way to
6-
* obtain one is through the smart constructor below, which runs the guard before
7-
* minting. So anything that later gets interpolated into a Redis key, a BullMQ
8-
* `jobId`, a `Symbol`, or raw DDL takes a branded parameter, and the compiler
9-
* refuses a raw `string` that skipped the check.
6+
* is PROOF that it already passed the identifier guard ({@link isSafeIdentifier}):
7+
* the ONLY way to obtain one is through the smart constructor below, which runs
8+
* the guard before minting. So anything that later gets interpolated into a Redis
9+
* key, a BullMQ `jobId`, a `Symbol`, or raw DDL takes a branded parameter, and the
10+
* compiler refuses a raw `string` that skipped the check.
1011
*
1112
* The single `as` assertion in {@link mint} is the ONE sanctioned type assertion
1213
* on this surface (the plan's E1 rule bans casts everywhere else). It is safe
13-
* precisely because `assertSafeIdentifier` has already thrown on anything that is
14-
* not `/^[a-zA-Z0-9_-]{1,63}$/` in canonical NFKC form.
14+
* precisely because `isSafeIdentifier` has already returned false — and `mint` has
15+
* thrown — on anything that is not `/^[a-zA-Z0-9_-]{1,63}$/` in canonical NFKC form.
1516
*
1617
* These are IDENTIFIER slugs (a plugin's short registration name, an authorizer
1718
* name, a capability key), NOT npm package names — a package name like
@@ -36,13 +37,23 @@ export type CapabilityKey = Branded<'CapabilityKey'>
3637
export type ModelName = Branded<'ModelName'>
3738

3839
/**
39-
* The one sanctioned assertion site. Runs the identifier guard (which THROWS on
40-
* anything unsafe, emitting `guard.tenant_identifier`) and only then mints the
41-
* brand. A dedicated `guard.plugin_extension_identifier` isthmus event is layered
42-
* on in the E3 hardening pass; today the shared identifier guard carries it.
40+
* The one sanctioned assertion site. Validates the raw string with the shared
41+
* identifier predicate and, on a reject, emits the dedicated plugin-surface guard
42+
* `guard.plugin_extension_identifier` (distinct from the tenant-DDL
43+
* `guard.tenant_identifier`, so an operator can tell a hostile plugin identifier
44+
* apart from a bad tenant id) and throws BEFORE minting the brand. The single `as`
45+
* below is reached only once the predicate has confirmed the value is safe (S6).
4346
*/
4447
function mint<B extends string>(raw: string, kind: string): Branded<B> {
45-
assertSafeIdentifier(raw, kind)
48+
if (!isSafeIdentifier(raw)) {
49+
emitIsthmusEvent('guard.plugin_extension_identifier', {
50+
metadata: { kind, value: String(raw).slice(0, 64) },
51+
})
52+
throw new Error(
53+
`Refusing to mint unsafe plugin ${kind} "${String(raw)}". ` +
54+
`Names must match /^[a-zA-Z0-9_-]{1,63}$/ in canonical (NFKC) form.`
55+
)
56+
}
4657
return raw as Branded<B>
4758
}
4859

0 commit comments

Comments
 (0)