Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .changeset/position-delegatable-phantom-lint-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
---
"@objectstack/spec": patch
---

fix(spec): `position.delegatable` no longer names a lint rule that was never written (#6628)

The JSDoc on the authorable `delegatable` key closed with

> so a delegatable position must never distribute an `adminScope`-carrying set
> (enforced by the `security-delegatable-admin-position` lint rule and the D12
> gate).

Only the second of those two enforcers exists. `security-delegatable-admin-position`
occurred **exactly once in the repository** — in that sentence. The security-domain
publish linter's rule table (`packages/lint/src/validate-security-posture.ts`) and its
twelve exported rule-id constants are the authority, and no delegatable/admin-position
rule is among them. The control that makes this a reading rather than a guess:
ADR-0091's *other* author-time rules did land — `security-grant-expired-at-authoring`
(D2) and `security-delegation-missing-reason` (D3, the same decision as `delegatable`)
are both present and both exported — so the absence is specific to this one rule, not
an artefact of the linter skipping ADR-0091.

The invariant itself is real and is enforced: `plugin-security`'s delegated-admin gate
implements the D12 containment check as step 6 of the self-service delegation path.
What was false is **when** it holds. The sentence promised an *author-time* gate, so an
author pairing `delegatable: true` with an `adminScope`-carrying permission set believed
`os lint` would stop them before shipping. It does not — the package publishes clean and
the mistake surfaces later, in a different package, as a runtime deny phrased as a fact
about the position rather than as a fix for the authoring error.

The JSDoc now names only the enforcer that exists and says plainly where it runs: the
D12 gate refuses the delegation at the moment a holder attempts it, denying with the
offending permission set named, so the failure an author will see is a delegation deny
at first use rather than a lint error. It also points at the one author-time rule
ADR-0091 D3 *does* have (`security-delegation-missing-reason`) and says what that one
actually checks, so "no lint rule for this" cannot be misread as "this invariant is
unenforced".

This is text only — a comment inside `position.zod.ts`, which `packages/spec` publishes
to npm via its `src/**/*.zod.ts` files entry, so the corrected prose reaches consumers
and AI authors reading the installed schema source. **`PositionSchema` accepts exactly
what it accepted before**; no key, default, or acceptance behaviour changed, and no
generated artifact moved.

Whether ADR-0091 D3 *should* grow an author-time rule for this combination is a separate
product decision and is deliberately not made here.
148 changes: 148 additions & 0 deletions packages/spec/src/identity/position-delegatable-enforcer.pin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#6628] The `delegatable` JSDoc may name a lint rule only if that rule exists.
*
* That JSDoc is an authoring surface, not a comment: it is the TSDoc an author
* (often an AI author, ADR-0033) hovers at the exact moment they type
* `delegatable:`. For a whole major it closed with
*
* "(enforced by the `security-delegatable-admin-position` lint rule and the
* D12 gate)"
*
* and the first of those two enforcers had never been written — the string
* occurred exactly once in the repository, in that sentence. The runtime half
* was real (`plugin-security`'s delegated-admin gate, step 6), so the invariant
* held; what was false was WHEN it holds. The sentence promised an author-time
* gate, so an author pairing `delegatable: true` with an `adminScope`-carrying
* set believed `os lint` would stop them. It does not: the package publishes
* clean and the mistake surfaces later, in a different package, as a runtime
* deny phrased as a fact about the position rather than as a fix for the
* authoring error.
*
* This is the `validate-security-posture.ts` header's own hazard one layer out.
* That file records how alias tolerance "silently downgraded a NAMED rejection
* into an inert branch — and an inert branch in a security linter reads, to the
* next author, as a gate that is watching (#4984, #5009, #5017)". A rule that is
* named but absent reads the same way, and is cheaper to write by accident:
* prose costs nothing to add and no compiler checks it.
*
* So the authority here is machine-readable, never a hand-copied list — the
* rule-id constants `packages/lint` actually exports, read off its `src/`
* directory the way `rule-id-barrel-exports.test.ts` (#5648) reads it. A gate
* name the rule table does not back turns this red.
*
* ⛔ Scope: the relation, not the wording. Rewording this JSDoc freely is fine —
* what it may not do is name a `security-*` rule that no rule file declares, or
* stop locating the containment check at runtime. The second half matters
* because "no author-time rule" is only safe to say next to "the D12 gate does
* enforce this, at delegation time"; drop that and the text overcorrects into
* implying the invariant is unenforced, which is the opposite lie.
*
* Deliberately NOT asserted: that no author-time rule exists. Whether ADR-0091
* D3 should grow one is a product decision (the finding left it open); if that
* rule is ever written, this pin stays green the moment the JSDoc names it,
* because the name will resolve against the same rule table.
*/

import { readFileSync, readdirSync } from 'node:fs';
import { dirname, join, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';

import { describe, it, expect } from 'vitest';

const HERE = dirname(fileURLToPath(import.meta.url));
/** …/packages/spec/src/identity → repo root */
const REPO_ROOT = resolve(HERE, '../../../..');
const LINT_SRC = join(REPO_ROOT, 'packages', 'lint', 'src');
const POSITION_SOURCE = join(HERE, 'position.zod.ts');

/** `export const NAME = 'security-…';` — how every security rule id is declared. */
const EXPORTED_SECURITY_RULE_ID = /^export const [A-Z][A-Z0-9_]* = '(security-[a-z0-9-]+)';\s*$/;

/**
* Every `security-*` rule id `packages/lint` declares, found by reading its
* `src/` directory rather than by naming files. A new security rule in a new
* file is therefore authoritative the moment it exists — the property a
* hand-maintained list here would quietly lose.
*/
function declaredSecurityRuleIds(): Set<string> {
const ids = new Set<string>();
for (const file of readdirSync(LINT_SRC)) {
if (!file.endsWith('.ts') || file.endsWith('.test.ts')) continue;
for (const line of readFileSync(join(LINT_SRC, file), 'utf8').split('\n')) {
const m = EXPORTED_SECURITY_RULE_ID.exec(line);
if (m) ids.add(m[1]);
}
}
return ids;
}

/** The JSDoc block attached to the authorable `delegatable` key. */
function delegatableDoc(): string {
const source = readFileSync(POSITION_SOURCE, 'utf8');
const key = source.indexOf('delegatable: z.boolean()');
expect(key, 'the `delegatable` key declaration moved — re-anchor this pin').toBeGreaterThan(-1);
const open = source.lastIndexOf('/**', key);
const close = source.indexOf('*/', open);
expect(open, 'no JSDoc block precedes `delegatable`').toBeGreaterThan(-1);
expect(close, 'unterminated JSDoc block').toBeLessThan(key);
return source.slice(open, close + 2);
}

/**
* The `security-*` rule ids a piece of prose names, minus the ones the rule
* table backs. Rule ids are matched by their backticked, multi-segment slug
* shape: `security-owd-alias` is a rule id, while the cloud product name
* `security-enterprise` (one segment, and never called a rule) is prose.
*/
function unbackedRuleIds(prose: string, backed: Set<string>): string[] {
const named = [...prose.matchAll(/`(security-[a-z0-9]+(?:-[a-z0-9]+)+)`/g)].map((m) => m[1]);
return [...new Set(named.filter((id) => !backed.has(id)))];
}

describe('`delegatable` JSDoc names only enforcers that exist (#6628)', () => {
it('reads a real rule table off `packages/lint`', () => {
const ids = declaredSecurityRuleIds();
// A floor, not an exact count — new security rules are expected. Its only
// job is to fail loudly if the extraction above stops finding anything,
// which would turn the self-test below vacuously green.
expect(ids.size).toBeGreaterThanOrEqual(12);
// The control the finding itself used: an ADR-0091 D3 author-time rule that
// DID land, proving the absence of the phantom was specific to that one
// rule and not an artefact of the linter skipping ADR-0091.
expect([...ids]).toContain('security-delegation-missing-reason');
});

it('would reject a gate name the rule table does not back (self-test)', () => {
// The historical sentence's shape, proving the predicate has teeth
// regardless of what the JSDoc currently says — without this, "no unbacked
// ids" below could pass simply because the prose stopped naming rules.
//
// The rule name here is deliberately SYNTHETIC rather than #6628's literal
// `security-delegatable-admin-position`. Whether ADR-0091 D3 should grow
// that author-time rule is an open product decision the finding declined to
// make; asserting its name is unbacked would quietly make this test the
// thing that breaks when someone implements it. What needs pinning is the
// predicate, not what any one unwritten rule would be called.
const before =
'so a delegatable position must never distribute an `adminScope`-carrying ' +
'set (enforced by the `security-no-such-rule-exists` lint rule and the D12 gate).';
expect(unbackedRuleIds(before, declaredSecurityRuleIds())).toEqual([
'security-no-such-rule-exists',
]);
});

it('names no rule that `packages/lint` does not declare', () => {
expect(unbackedRuleIds(delegatableDoc(), declaredSecurityRuleIds())).toEqual([]);
});

it('still locates the D12 containment check at runtime', () => {
const doc = delegatableDoc();
// Both halves, together: the gate that does enforce it, and WHEN. Naming
// D12 without placing it at runtime is the sentence this pin was written
// for; placing it at runtime without naming D12 reads as unenforced.
expect(doc).toContain('D12');
expect(doc).toMatch(/runtime/i);
});
});
16 changes: 13 additions & 3 deletions packages/spec/src/identity/position.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,19 @@ export const PositionSchema = lazySchema(() => strictObject(
* positions (an approver going on leave) opt in; admin-ish positions do
* NOT — delegating administration would bypass the D12 containment gate,
* so a delegatable position must never distribute an `adminScope`-carrying
* set (enforced by the `security-delegatable-admin-position` lint rule and
* the D12 gate). A grant that itself arrived via delegation is not
* re-delegatable (chains are cut).
* set. A grant that itself arrived via delegation is not re-delegatable
* (chains are cut).
*
* That invariant IS enforced — but at RUNTIME, not at authoring time. The
* D12 containment gate (`plugin-security`'s delegated-admin gate, step 6 of
* the self-service delegation path) refuses the delegation the moment a
* holder attempts it, denying with the offending permission set named. No
* lint rule checks the combination, so a package pairing `delegatable: true`
* with an `adminScope`-carrying set publishes clean and `os lint` stays
* green: what you will see is a delegation deny at first use, not an
* author-time error. (The one author-time rule ADR-0091 D3 does have,
* `security-delegation-missing-reason`, checks something else — that a
* seeded delegation row carries its dual-audit reason.)
*/
delegatable: z.boolean().default(false).describe(
'ADR-0091 D3: holders may self-service delegate this position, time-boxed (default false).',
Expand Down
Loading