mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
feat(app-blocks): enforce a justification for sensitive scopes at manifest submit (#3451)
* feat(app-blocks): enforce a justification for sensitive scopes at manifest submit A manifest that declares a SENSITIVE block scope (one that can spend or read the viewer's Buzz, read their private data, or write data other users see) must now carry a non-empty scopeJustifications entry for that scope, or the manifest is rejected. scopeJustifications was previously optional metadata that was never required; this makes it required for the sensitive subset. Enforced in the shared BlockManifestValidator.validate, so it covers every submit path — the CLI zip submit (/api/v1/blocks/submit-version) and the web editor (blocks.updateManifest) both funnel through validateSubmission -> validate. The on-site mod-review checklist now auto-derives the "permissions justified" item, and SENSITIVE_BLOCK_SCOPES is promoted from a presentation-only classification to one that also gates manifest validity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(app-blocks): scope sensitive-scope-justification enforcement to SUBMIT only The enforcement was leaking to the moderator APPROVE path: approve re-validates the stored submitted manifest via validateSubmission (H-4 invariant), so a legacy pending request submitted before this rule shipped — sensitive scope, no scopeJustifications (valid under the old rules) — would throw on approve and become un-approvable. Thread a ManifestValidationOptions param (enforceSensitiveScopeJustification, default true) through validate + validateSubmission. The 3 genuine submit callers (git-push, developer manifest API, blocks.updateManifest) keep the default (ON); the approve re-validation passes false, exempting ONLY this rule — every other manifest check still runs on approve. No bypass: a post-deploy submission already passed the submit gate (carries justifications), and nothing reaches the approve queue without passing submit first, so grandfathering legacy pending requests is safe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
@@ -92,7 +92,7 @@
|
||||
},
|
||||
"scopeJustifications": {
|
||||
"type": "object",
|
||||
"description": "OPTIONAL per-scope justification: a map of scope-id → free-text rationale explaining WHY the app needs that permission, shown to the moderator during review. Backward-compatible — omit it and the manifest stays valid; `scopes` is unchanged. Every key MUST be a scope also present in `scopes` (justifications for scopes you don't request are rejected). Each value is a non-empty string of at most 500 characters. NOTE: this captures the developer's STATED rationale only; the platform does not verify the claims.",
|
||||
"description": "Per-scope justification: a map of scope-id → free-text rationale explaining WHY the app needs that permission, shown to the moderator during review. REQUIRED for SENSITIVE scopes — any declared scope that can spend or read the viewer's Buzz, read the viewer's private data, or write data other users see (e.g. `ai:write:budgeted`, `social:tip:self`, `buzz:read:self`, `collections:read:private`, `apps:storage:shared:write`) MUST carry a non-empty justification here, or the manifest is rejected at submit time. OPTIONAL for non-sensitive scopes — omit those and the manifest stays valid. Every key MUST be a scope also present in `scopes` (justifications for scopes you don't request are rejected). Each value is a non-empty string of at most 500 characters. The requirement is enforced imperatively by the manifest validator (not expressed as JSON-Schema conditionals here). NOTE: the justification captures the developer's STATED rationale only; the platform does not verify the truth of the claims.",
|
||||
"additionalProperties": {
|
||||
"type": "string",
|
||||
"minLength": 1,
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
getOffsiteReviewChecklist,
|
||||
getOnsiteReviewChecklist,
|
||||
getReviewChecklist,
|
||||
unjustifiedSensitiveBlockScopes,
|
||||
unjustifiedSensitiveScopeKeys,
|
||||
type OffsiteChecklistData,
|
||||
} from '../offsiteReviewChecklist';
|
||||
@@ -190,6 +191,71 @@ describe('getOffsiteReviewChecklist — connect sensitive-scope item (PR3)', ()
|
||||
});
|
||||
});
|
||||
|
||||
describe('getOnsiteReviewChecklist — auto-derived scopes item (block sensitive scopes)', () => {
|
||||
const scopesStatus = (data?: Parameters<typeof getOnsiteReviewChecklist>[0]) =>
|
||||
getOnsiteReviewChecklist(data).find((i) => i.id === 'scopes');
|
||||
|
||||
it('with NO data the scopes item stays a mod-judgment todo (backward compatible)', () => {
|
||||
expect(scopesStatus()?.status).toBe('todo');
|
||||
});
|
||||
|
||||
it('a declared sensitive scope WITHOUT a justification → warn, hint names the scope', () => {
|
||||
const item = scopesStatus({ scopes: ['apps:storage:shared:write'] });
|
||||
expect(item?.status).toBe('warn');
|
||||
expect(item?.hint).toContain('apps:storage:shared:write');
|
||||
});
|
||||
|
||||
it('every declared sensitive scope justified → ok, hint surfaces the justification', () => {
|
||||
const item = scopesStatus({
|
||||
scopes: ['apps:storage:shared:write'],
|
||||
scopeJustifications: {
|
||||
'apps:storage:shared:write': 'We persist shared gallery entries.',
|
||||
},
|
||||
});
|
||||
expect(item?.status).toBe('ok');
|
||||
expect(item?.hint).toContain('apps:storage:shared:write');
|
||||
expect(item?.hint).toContain('We persist shared gallery entries.');
|
||||
});
|
||||
|
||||
it('data present but NO sensitive scopes declared → ok (nothing to justify)', () => {
|
||||
const item = scopesStatus({ scopes: ['models:read:self'] });
|
||||
expect(item?.status).toBe('ok');
|
||||
});
|
||||
|
||||
it('multiple sensitive scopes, one justified one not → warn, hint names the unjustified one', () => {
|
||||
const item = scopesStatus({
|
||||
scopes: ['collections:read:private', 'apps:storage:shared:write'],
|
||||
scopeJustifications: { 'collections:read:private': 'We read private collections.' },
|
||||
});
|
||||
expect(item?.status).toBe('warn');
|
||||
expect(item?.hint).toContain('apps:storage:shared:write');
|
||||
});
|
||||
});
|
||||
|
||||
describe('unjustifiedSensitiveBlockScopes', () => {
|
||||
it('returns declared sensitive block scopes lacking a non-empty justification', () => {
|
||||
expect(
|
||||
unjustifiedSensitiveBlockScopes({
|
||||
scopes: ['models:read:self', 'collections:read:private', 'apps:storage:shared:write'],
|
||||
scopeJustifications: { 'collections:read:private': 'ok' }, // shared:write missing
|
||||
})
|
||||
).toEqual(['apps:storage:shared:write']);
|
||||
});
|
||||
|
||||
it('empty when no sensitive scope is declared', () => {
|
||||
expect(unjustifiedSensitiveBlockScopes({ scopes: ['models:read:self'] })).toEqual([]);
|
||||
});
|
||||
|
||||
it('whitespace-only justification counts as missing', () => {
|
||||
expect(
|
||||
unjustifiedSensitiveBlockScopes({
|
||||
scopes: ['apps:storage:shared:write'],
|
||||
scopeJustifications: { 'apps:storage:shared:write': ' ' },
|
||||
})
|
||||
).toEqual(['apps:storage:shared:write']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('unjustifiedSensitiveScopeKeys', () => {
|
||||
it('returns the enum-keys of sensitive requested scopes lacking a non-empty justification', () => {
|
||||
expect(
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { validateExternalUrl } from '~/server/schema/blocks/external-app.schema';
|
||||
import { isSensitiveBlockScope } from '~/shared/constants/block-scope.constants';
|
||||
import {
|
||||
SENSITIVE_TOKEN_SCOPES,
|
||||
tokenScopeMaskToList,
|
||||
@@ -77,13 +78,70 @@ export function unjustifiedSensitiveScopeKeys(data: {
|
||||
.map(({ key }) => key);
|
||||
}
|
||||
|
||||
/** The manifest facts the ON-SITE (App Block) scopes checklist item derives from. */
|
||||
export type OnsiteChecklistData = {
|
||||
/** The manifest's declared block scopes (`manifest.scopes`). */
|
||||
scopes?: string[] | null;
|
||||
/** The manifest's declared per-scope justifications (`manifest.scopeJustifications`). */
|
||||
scopeJustifications?: Record<string, string> | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* The declared BLOCK sensitive scopes (money / private data / cross-user writes)
|
||||
* that have NO non-empty justification. Mirrors `unjustifiedSensitiveScopeKeys`
|
||||
* but keyed on the App-Block sensitive set (`isSensitiveBlockScope`) + the
|
||||
* manifest's `scopeJustifications` — NOT the connect token bitmask. Empty when
|
||||
* no sensitive scope is declared or every one is justified. Since the manifest
|
||||
* validator now ENFORCES a justification for these at submit time, an approved
|
||||
* app returns `[]` here — a non-empty result flags a legacy/unenforced manifest.
|
||||
*/
|
||||
export function unjustifiedSensitiveBlockScopes(data: OnsiteChecklistData): string[] {
|
||||
const scopes = data.scopes ?? [];
|
||||
const justifications = data.scopeJustifications ?? {};
|
||||
return [...new Set(scopes)]
|
||||
.filter((scope) => isSensitiveBlockScope(scope))
|
||||
.filter((scope) => {
|
||||
const raw = justifications[scope];
|
||||
return !(typeof raw === 'string' && raw.trim().length > 0);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The deep ON-SITE (App Block) review checklist. These items are STATIC reminders
|
||||
* of the code/bundle review the mod performs in the existing modal panels — the
|
||||
* off-site content checklist deliberately OMITS every one of them (there is no
|
||||
* bundle / manifest / scopes / code to read for an external-link app).
|
||||
*/
|
||||
export function getOnsiteReviewChecklist(): ReviewChecklistItem[] {
|
||||
export function getOnsiteReviewChecklist(data?: OnsiteChecklistData): ReviewChecklistItem[] {
|
||||
// The `scopes` item AUTO-DERIVES from the manifest when scope data is supplied
|
||||
// (mirroring the off-site connect variant, but keyed on the BLOCK sensitive set
|
||||
// + the manifest's `scopeJustifications`). Submit now ENFORCES a justification
|
||||
// for every sensitive scope, so an approved app shows `ok`; a legacy/unenforced
|
||||
// manifest missing one surfaces as `warn`. With no data the item stays a `todo`
|
||||
// reminder (unchanged) — and the hint still surfaces the justifications to the
|
||||
// mod so they can confirm each stated rationale is truthful.
|
||||
const declaredSensitive = data ? (data.scopes ?? []).filter(isSensitiveBlockScope) : [];
|
||||
const unjustifiedSensitive = data ? unjustifiedSensitiveBlockScopes(data) : [];
|
||||
const justifications = data?.scopeJustifications ?? {};
|
||||
const scopesStatus: ReviewChecklistItemStatus = !data
|
||||
? 'todo'
|
||||
: unjustifiedSensitive.length > 0
|
||||
? 'warn'
|
||||
: 'ok';
|
||||
const scopesHint = !data
|
||||
? 'Every requested scope is needed for the stated functionality.'
|
||||
: unjustifiedSensitive.length > 0
|
||||
? `Missing a justification for sensitive scope(s): ${unjustifiedSensitive.join(
|
||||
', '
|
||||
)} — submit should have blocked this; do not approve until justified.`
|
||||
: declaredSensitive.length > 0
|
||||
? `Sensitive permissions justified — ${declaredSensitive
|
||||
.map((scope) => {
|
||||
const j = justifications[scope];
|
||||
return typeof j === 'string' && j.trim().length > 0 ? `${scope}: "${j.trim()}"` : scope;
|
||||
})
|
||||
.join('; ')}. Confirm each rationale is truthful.`
|
||||
: 'No sensitive permissions requested; confirm the remaining scopes are needed for the stated functionality.';
|
||||
return [
|
||||
{
|
||||
id: 'code-diff',
|
||||
@@ -106,8 +164,8 @@ export function getOnsiteReviewChecklist(): ReviewChecklistItem[] {
|
||||
{
|
||||
id: 'scopes',
|
||||
label: 'Requested permissions justified',
|
||||
hint: 'Every requested scope is needed for the stated functionality.',
|
||||
status: 'todo',
|
||||
hint: scopesHint,
|
||||
status: scopesStatus,
|
||||
},
|
||||
{
|
||||
id: 'screenshots',
|
||||
|
||||
@@ -843,6 +843,192 @@ describe('BlockManifestValidator', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ENFORCEMENT: a declared SENSITIVE scope (money / private data / cross-user
|
||||
// write) now REQUIRES a non-empty justification — this used to be optional
|
||||
// metadata. Runs in `validate`, so it covers every submit path (CLI
|
||||
// submit-version + web updateManifest both funnel through validateSubmission →
|
||||
// validate). `apps:storage:shared:write` / `collections:read:private` are
|
||||
// sensitive AND SKIP_OAUTH_CHECK, so they need no extra OAuth bit; the
|
||||
// AI context covers `ai:write:budgeted` (requires AIServicesWrite).
|
||||
describe('scopeJustifications — ENFORCED for sensitive scopes', () => {
|
||||
const AI_APP_CTX = {
|
||||
allowedScopes: TokenScope.ModelsRead | TokenScope.AIServicesWrite,
|
||||
allowedOrigins: ['https://blocks.civitai.com'],
|
||||
};
|
||||
|
||||
it('REJECTS a sensitive scope with NO scopeJustifications (previously accepted)', () => {
|
||||
// ai:write:budgeted can spend the viewer's Buzz — a justification is now required.
|
||||
const manifest = { ...VALID_MANIFEST, scopes: ['ai:write:budgeted'] };
|
||||
const result = BlockManifestValidator.validate(manifest, AI_APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
expect(
|
||||
result.errors.some(
|
||||
(e) => e.includes('sensitive scopes require a justification') && e.includes('ai:write:budgeted')
|
||||
)
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('REJECTS a sensitive scope when scopeJustifications is present but omits its key', () => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['models:read:self', 'apps:storage:shared:write'],
|
||||
// Justifies the non-sensitive scope but not the sensitive one.
|
||||
scopeJustifications: { 'models:read:self': 'We render the page model.' },
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
expect(
|
||||
result.errors.some(
|
||||
(e) =>
|
||||
e.includes('sensitive scopes require a justification') &&
|
||||
e.includes('apps:storage:shared:write')
|
||||
)
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it.each<[string, string]>([
|
||||
['', 'empty string'],
|
||||
[' ', 'whitespace-only'],
|
||||
['\t\n', 'tabs/newlines only'],
|
||||
])('REJECTS a sensitive scope justified with a %j (%s) value', (value) => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['apps:storage:shared:write'],
|
||||
scopeJustifications: { 'apps:storage:shared:write': value },
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
expect(result.errors.some((e) => e.includes('apps:storage:shared:write'))).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('ACCEPTS a sensitive scope WITH a non-empty justification', () => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['apps:storage:shared:write'],
|
||||
scopeJustifications: {
|
||||
'apps:storage:shared:write': 'We persist shared gallery entries other users can see.',
|
||||
},
|
||||
};
|
||||
expect(BlockManifestValidator.validate(manifest, APP_CTX)).toEqual({ valid: true });
|
||||
});
|
||||
|
||||
it('ACCEPTS a NON-sensitive scope with no justification (unchanged)', () => {
|
||||
const manifest = { ...VALID_MANIFEST, scopes: ['models:read:self'] };
|
||||
expect(BlockManifestValidator.validate(manifest, APP_CTX)).toEqual({ valid: true });
|
||||
});
|
||||
|
||||
it('lists ALL unjustified sensitive scopes when several are missing', () => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['collections:read:private', 'apps:storage:shared:write'],
|
||||
// Neither sensitive scope justified.
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
const err = result.errors.find((e) =>
|
||||
e.includes('sensitive scopes require a justification')
|
||||
);
|
||||
expect(err).toBeDefined();
|
||||
expect(err).toContain('collections:read:private');
|
||||
expect(err).toContain('apps:storage:shared:write');
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects naming ONLY the unjustified sensitive scope when some are justified', () => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['collections:read:private', 'apps:storage:shared:write'],
|
||||
scopeJustifications: { 'collections:read:private': 'We read the viewer’s private collections.' },
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
const err = result.errors.find((e) =>
|
||||
e.includes('sensitive scopes require a justification')
|
||||
);
|
||||
expect(err).toBeDefined();
|
||||
expect(err).toContain('apps:storage:shared:write');
|
||||
expect(err).not.toContain('collections:read:private');
|
||||
}
|
||||
});
|
||||
|
||||
it('still applies the shape rules (keys ⊆ scopes, ≤500, non-empty) alongside enforcement', () => {
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['apps:storage:shared:write'],
|
||||
scopeJustifications: {
|
||||
'apps:storage:shared:write': 'ok reason',
|
||||
// A justification for a scope NOT declared — still rejected by the shape rule.
|
||||
'user:read:self': 'dangling',
|
||||
},
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX);
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
expect(
|
||||
result.errors.some((e) => e.includes('user:read:self') && e.includes('not in the manifest'))
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
// SUBMIT-vs-APPROVE: the enforcement is SUBMIT-only. The moderator approve
|
||||
// re-validation passes `enforceSensitiveScopeJustification:false` so a LEGACY
|
||||
// pending request (sensitive scope, no justification, valid under the old
|
||||
// rules) stays approvable — but ALL OTHER manifest checks still run on approve.
|
||||
describe('SUBMIT-only (approve exemption)', () => {
|
||||
it('SUBMIT context (default) still REJECTS a sensitive scope with no justification', () => {
|
||||
const manifest = { ...VALID_MANIFEST, scopes: ['apps:storage:shared:write'] };
|
||||
// Default (no opts) = genuine submit = enforce.
|
||||
expect(BlockManifestValidator.validate(manifest, APP_CTX).valid).toBe(false);
|
||||
});
|
||||
|
||||
it('APPROVE context (enforceSensitiveScopeJustification:false) does NOT reject on the sensitive-scope rule', () => {
|
||||
const manifest = { ...VALID_MANIFEST, scopes: ['apps:storage:shared:write'] };
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX, {
|
||||
enforceSensitiveScopeJustification: false,
|
||||
});
|
||||
expect(result).toEqual({ valid: true });
|
||||
});
|
||||
|
||||
it('APPROVE context is exempted through validateSubmission too', async () => {
|
||||
const manifest = { ...VALID_MANIFEST, scopes: ['collections:read:private'] };
|
||||
const result = await BlockManifestValidator.validateSubmission(manifest, APP_CTX, {
|
||||
enforceSensitiveScopeJustification: false,
|
||||
});
|
||||
expect(result).toEqual({ valid: true });
|
||||
});
|
||||
|
||||
it('APPROVE context STILL enforces every OTHER manifest rule (only this one is exempt)', () => {
|
||||
// A sensitive scope with no justification (would pass the exempted rule)
|
||||
// but also an UNKNOWN scope + a non-https iframe.src — both must still fail.
|
||||
const manifest = {
|
||||
...VALID_MANIFEST,
|
||||
scopes: ['apps:storage:shared:write', 'models:read:all'],
|
||||
iframe: { ...VALID_MANIFEST.iframe, src: 'http://blocks.civitai.com/test' },
|
||||
};
|
||||
const result = BlockManifestValidator.validate(manifest, APP_CTX, {
|
||||
enforceSensitiveScopeJustification: false,
|
||||
});
|
||||
expect(result.valid).toBe(false);
|
||||
if (!result.valid) {
|
||||
expect(result.errors.some((e) => e.includes('not a known block scope'))).toBe(true);
|
||||
expect(result.errors.some((e) => e.includes('iframe.src'))).toBe(true);
|
||||
// The exempted rule did NOT contribute an error.
|
||||
expect(
|
||||
result.errors.some((e) => e.includes('sensitive scopes require a justification'))
|
||||
).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// SUBMISSION gate: validateSubmission = the synchronous shape/security checks
|
||||
// PLUS the accurate ReDoS + input-bound gate on settings-field patterns. This
|
||||
// is what every manifest-SUBMISSION path calls (git-push webhook, developer
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import {
|
||||
isKnownBlockScope,
|
||||
isSensitiveBlockScope,
|
||||
validateBlockScopesAgainstOauthClient,
|
||||
} from '~/shared/constants/block-scope.constants';
|
||||
import { isKnownSlotId, isPageSlot } from '~/shared/constants/slot-registry';
|
||||
@@ -18,6 +19,27 @@ import { SCOPE_JUSTIFICATION_MAX_LENGTH } from '@civitai/auth/token-scope';
|
||||
|
||||
type ValidationResult = { valid: true } | { valid: false; errors: string[] };
|
||||
|
||||
/**
|
||||
* Options controlling WHICH validation rules run. Defaults to full enforcement
|
||||
* (every rule on); a caller passes this only to RELAX a rule for a specific
|
||||
* context. Currently the sole knob exempts the sensitive-scope-justification
|
||||
* enforcement on the moderator APPROVE re-validation.
|
||||
*/
|
||||
export type ManifestValidationOptions = {
|
||||
/**
|
||||
* Enforce that every declared SENSITIVE scope carries a non-empty
|
||||
* justification. Default `true` (the genuine submit / new-version paths). The
|
||||
* moderator APPROVE re-validation passes `false` so a LEGACY pending request —
|
||||
* submitted before this rule shipped, with a sensitive scope and no
|
||||
* justification — stays approvable (grandfathered). No bypass is created: a
|
||||
* post-deploy submission already passed this gate at submit time, so
|
||||
* re-checking it on approve is redundant, and nothing can reach the approve
|
||||
* queue post-deploy without first passing the submit gate. ALL OTHER
|
||||
* validation still runs on approve.
|
||||
*/
|
||||
enforceSensitiveScopeJustification?: boolean;
|
||||
};
|
||||
|
||||
interface RawManifest {
|
||||
blockId?: unknown;
|
||||
version?: unknown;
|
||||
@@ -301,7 +323,11 @@ export class BlockManifestValidator {
|
||||
// Back-compat overload: the existing test suite passes a bitmask number.
|
||||
// Real callers pass the AppContext shape (with allowedOrigins) so the
|
||||
// H8 binding check actually runs.
|
||||
static validate(manifest: unknown, app: AppContext | number): ValidationResult {
|
||||
static validate(
|
||||
manifest: unknown,
|
||||
app: AppContext | number,
|
||||
opts?: ManifestValidationOptions
|
||||
): ValidationResult {
|
||||
const ctx: AppContext =
|
||||
typeof app === 'number'
|
||||
? { allowedScopes: app, allowedOrigins: [] }
|
||||
@@ -460,6 +486,49 @@ export class BlockManifestValidator {
|
||||
}
|
||||
}
|
||||
|
||||
// ENFORCEMENT (was presentation-only): every declared SENSITIVE scope — the
|
||||
// subset that can spend/read the viewer's Buzz, read their PRIVATE data, or
|
||||
// write data other users see (see SENSITIVE_BLOCK_SCOPES) — MUST carry a
|
||||
// non-empty justification. `scopeJustifications` used to be fully optional;
|
||||
// for sensitive scopes it is now REQUIRED, so a moderator always sees WHY an
|
||||
// elevated-risk permission was requested. An entirely-absent
|
||||
// `scopeJustifications` (previously valid) now fails when any sensitive scope
|
||||
// is declared. Reuses the single-sourced sensitive set (never re-hardcodes
|
||||
// it), and runs for every SUBMIT path because both the CLI submit-version and
|
||||
// the web `updateManifest` funnel through `validateSubmission` → this
|
||||
// `validate`.
|
||||
//
|
||||
// SUBMIT-ONLY (default on): the moderator APPROVE re-validation passes
|
||||
// `enforceSensitiveScopeJustification:false` so a LEGACY pending request
|
||||
// (submitted before this shipped, no justification) stays approvable. Only
|
||||
// THIS rule is exempted on approve — every other check above still runs.
|
||||
if (opts?.enforceSensitiveScopeJustification !== false && Array.isArray(m.scopes)) {
|
||||
const justifications =
|
||||
m.scopeJustifications &&
|
||||
typeof m.scopeJustifications === 'object' &&
|
||||
!Array.isArray(m.scopeJustifications)
|
||||
? (m.scopeJustifications as Record<string, unknown>)
|
||||
: {};
|
||||
const unjustifiedSensitive = [
|
||||
...new Set(
|
||||
(m.scopes as unknown[])
|
||||
.filter((s): s is string => typeof s === 'string')
|
||||
.filter((scope) => isSensitiveBlockScope(scope))
|
||||
.filter((scope) => {
|
||||
const raw = justifications[scope];
|
||||
return !(typeof raw === 'string' && raw.trim().length > 0);
|
||||
})
|
||||
),
|
||||
];
|
||||
if (unjustifiedSensitive.length > 0) {
|
||||
errors.push(
|
||||
`sensitive scopes require a justification — add a non-empty scopeJustifications entry for: ${unjustifiedSensitive.join(
|
||||
', '
|
||||
)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// H8: build the app-bound origin allowlist. Manifest URLs (iframe.src,
|
||||
// assetBundleUrl) must be on origins the registrant actually controls,
|
||||
// not arbitrary HTTPS endpoints. Without this binding, anyone with
|
||||
@@ -718,9 +787,10 @@ export class BlockManifestValidator {
|
||||
*/
|
||||
static async validateSubmission(
|
||||
manifest: unknown,
|
||||
app: AppContext | number
|
||||
app: AppContext | number,
|
||||
opts?: ManifestValidationOptions
|
||||
): Promise<ValidationResult> {
|
||||
const base = this.validate(manifest, app);
|
||||
const base = this.validate(manifest, app, opts);
|
||||
const errors: string[] = base.valid ? [] : [...base.errors];
|
||||
|
||||
const settings =
|
||||
|
||||
@@ -2119,7 +2119,16 @@ export async function approveRequest(params: ApproveRequestParams): Promise<Appr
|
||||
o.toLowerCase()
|
||||
),
|
||||
};
|
||||
const validation = await BlockManifestValidator.validateSubmission(manifest, validationCtx);
|
||||
// APPROVE is a re-validation of the ALREADY-SUBMITTED manifest, not a new
|
||||
// submit — exempt ONLY the new sensitive-scope-justification enforcement so a
|
||||
// LEGACY pending request (submitted before that rule shipped, sensitive scope
|
||||
// + no justification) stays approvable. Every OTHER manifest check still runs
|
||||
// here (the H-4 invariant). A post-deploy submission already passed this gate
|
||||
// at submit time, and nothing reaches the approve queue without the submit
|
||||
// gate first, so skipping the re-check creates no bypass.
|
||||
const validation = await BlockManifestValidator.validateSubmission(manifest, validationCtx, {
|
||||
enforceSensitiveScopeJustification: false,
|
||||
});
|
||||
if (!validation.valid) {
|
||||
throw new Error(
|
||||
`Invalid manifest — cannot approve. The git-push webhook would reject this manifest with the same errors and the build chain would not run. ` +
|
||||
|
||||
@@ -149,10 +149,15 @@ export function isKnownBlockScope(scope: string): scope is BlockScopeString {
|
||||
* - read the viewer's PRIVATE data (`collections:read:private`)
|
||||
* - write data OTHER users see (`apps:storage:shared:write`)
|
||||
*
|
||||
* This is a PRESENTATION classification only — it changes how a scope is
|
||||
* displayed, never whether it is granted/enforced (that stays with the
|
||||
* server-side per-op gates + consent grant). Keeping it a set (not a per-scope
|
||||
* flag on the map) keeps the enforcement map and the UI emphasis decoupled.
|
||||
* This set does two things. (1) PRESENTATION — it drives the distinct,
|
||||
* warning-styled emphasis wherever scopes are surfaced. (2) ENFORCEMENT — it
|
||||
* now also gates MANIFEST VALIDITY: at submit time the manifest validator
|
||||
* REQUIRES a non-empty `scopeJustifications` entry for every declared sensitive
|
||||
* scope (see `block-manifest-validator.service.ts`), so a moderator always sees
|
||||
* WHY an elevated-risk permission was requested. It does NOT change whether a
|
||||
* granted scope is enforced at call time — that stays with the server-side
|
||||
* per-op gates + consent grant. Keeping it a set (not a per-scope flag on the
|
||||
* map) keeps the enforcement map and this classification decoupled.
|
||||
*
|
||||
* INVARIANT (guarded by a test): every entry must be a currently-known scope in
|
||||
* `BLOCK_SCOPE_TO_OAUTH_BIT`. If a scope is renamed/removed (as
|
||||
|
||||
Reference in New Issue
Block a user