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
142 changes: 142 additions & 0 deletions __tests__/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,37 @@ describe( 'validateIntegration', () => {
expect( rule3?.details?.join( '\n' ) ).toMatch( /constant_name is malformed/ );
} );

it( 'rejects a YAML alias bomb manifest fast instead of hanging (rule 3)', () => {
const root = join( dir, 'manifest-alias-bomb' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
writeFileSync(
join( root, 'vip-manifest.yaml' ),
[
'manifest_version: 1',
'a: &a ["x","x","x","x","x","x","x","x","x"]',
'b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]',
'c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]',
'd: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]',
'e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]',
'f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]',
'g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]',
'h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]',
'i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]',
].join( '\n' )
);

const start = Date.now();
const rule3 = validateIntegration( root ).results.find(
result => result.id === 'handoff-manifest'
);
// The old code walked the expanded alias tree and burned tens of seconds;
// rejecting at parse time must return effectively instantly.
expect( Date.now() - start ).toBeLessThan( 2000 );
expect( rule3?.status ).toBe( 'fail' );
expect( rule3?.message ).toMatch( /could not be read as YAML/ );
} );

it( 'fails rule 3 when manifest_kind is wrong', () => {
const root = join( dir, 'manifest-kind' );
mkdirSync( root, { recursive: true } );
Expand Down Expand Up @@ -747,6 +778,117 @@ describe( 'validateIntegration', () => {
expect( rule7?.message ).toMatch( /PHP 8\.5/ );
} );

it( 'accepts a PHP matrix written as a YAML flow array for rule 7', () => {
const root = join( dir, 'php-flow-array' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
writeFileSync(
join( root, '.github', 'workflows', 'unit-tests.yml' ),
[
'jobs:',
' test:',
' strategy:',
' matrix:',
' wp: [6.9, 7.0]',
' php: [8.2, 8.3, 8.4, 8.5]',
].join( '\n' )
);

expect( statusById( root )[ 'compatibility-matrix' ] ).toBe( 'pass' );
} );

it( 'accepts a PHP matrix written as a YAML block sequence for rule 7', () => {
const root = join( dir, 'php-block-sequence' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
writeFileSync(
join( root, '.github', 'workflows', 'unit-tests.yml' ),
[
'jobs:',
' test:',
' strategy:',
' matrix:',
' wp: [6.9, 7.0]',
' php-version:',
" - '8.2'",
" - '8.3'",
" - '8.4'",
" - '8.5'",
].join( '\n' )
);

expect( statusById( root )[ 'compatibility-matrix' ] ).toBe( 'pass' );
} );

it( 'fails rule 7 when 6.9/7.0 sit against a non-WordPress key (no WP evidence)', () => {
const root = join( dir, 'php-wp-unscoped' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
// `node: [6.9, 7.0]` is not WordPress evidence — Rule 7 must not read it as
// WP coverage just because the tokens appear somewhere in the workflow.
writeFileSync(
join( root, '.github', 'workflows', 'unit-tests.yml' ),
[
'jobs:',
' test:',
' strategy:',
' matrix:',
' node: [6.9, 7.0]',
' php: [8.2, 8.3, 8.4, 8.5]',
].join( '\n' )
);

const rule7 = validateIntegration( root ).results.find(
result => result.id === 'compatibility-matrix'
);
expect( rule7?.status ).toBe( 'fail' );
expect( rule7?.message ).toMatch( /WordPress 6\.9/ );
expect( rule7?.message ).toMatch( /WordPress 7\.0/ );
} );

it( 'accepts the `php-versions` (plural) matrix key for rule 7', () => {
const root = join( dir, 'php-versions-plural' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
writeFileSync(
join( root, '.github', 'workflows', 'unit-tests.yml' ),
[
'jobs:',
' test:',
' strategy:',
' matrix:',
' wp: [6.9, 7.0]',
" php-versions: ['8.2', '8.3', '8.4', '8.5']",
].join( '\n' )
);

expect( statusById( root )[ 'compatibility-matrix' ] ).toBe( 'pass' );
} );

it( 'does not count PHP versions that only appear in a trailing comment', () => {
const root = join( dir, 'php-comment' );
mkdirSync( root, { recursive: true } );
scaffoldConformant( root );
// 8.5 only appears in a comment — it must not count as coverage.
writeFileSync(
join( root, '.github', 'workflows', 'unit-tests.yml' ),
[
'jobs:',
' test:',
' strategy:',
' matrix:',
' wp: [6.9, 7.0]',
' php: [8.2, 8.3, 8.4] # 8.5 dropped for now',
].join( '\n' )
);

const rule7 = validateIntegration( root ).results.find(
result => result.id === 'compatibility-matrix'
);
expect( rule7?.status ).toBe( 'fail' );
expect( rule7?.message ).toMatch( /PHP 8\.5/ );
} );

it( 'warns (not passes) rule 7 when a structured compatibility exception is claimed', () => {
const root = join( dir, 'compat-exception' );
mkdirSync( root, { recursive: true } );
Expand Down
4 changes: 2 additions & 2 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ Checks if the integration meets the wpvip guidelines. All checks are **static**

One rule validates the **handoff manifest** (`vip-manifest.yaml`) — the single file a partner fills in so VIP can register and load the integration from the manifest alone. `manifest.ts` parses it and validates it against `manifest.schema.ts` (a JSON Schema, compiled with Ajv) — the single source of truth for the manifest's fields and constraints. It is a presence-and-shape check that every field VIP consumes (identity, documentation, plugin runtime, the runtime-config schema, telemetry, and release metadata) is present and well-formed, not a check that the values are correct. The Starter Kit ships an identical `vip-manifest.schema.json` so partners validate against the same contract in their editor.

Beyond the schema, the same rule enforces two things a raw schema can't. First, it fails while any field still holds the `MANIFEST_PLACEHOLDER` sentinel that `init` leaves in the partner-only fields (contact, docs URLs), so a partner cannot submit a half-filled scaffold. Second, it cross-checks the config keys the plugin declares (`Config::REQUIRED_FIELDS` / `SENSITIVE_FIELDS`) against the manifest's `runtime_config.fields`, so a config field the code reads from the constant can't be missing from or mis-typed in the manifest. That cross-check is deterministic for integrations following the Starter Kit Config convention and skipped for any plugin that declares neither array.
Beyond the schema, the same rule adds two checks a raw schema can't. First, it **fails** while any field still holds the `MANIFEST_PLACEHOLDER` sentinel that `init` leaves in the partner-only fields (contact, docs URLs), so a partner cannot submit a half-filled scaffold — this is the only one of the two that blocks conformance. Second, it cross-checks the config keys the plugin declares (`Config::REQUIRED_FIELDS` / `SENSITIVE_FIELDS`) against the manifest's `runtime_config.fields` and **warns** (does not fail) when a field is missing from or mis-typed in the manifest. That cross-check is best-effort — the plugin's contract is read heuristically from the PHP source, so it can't be certain it matched the real `Config` class, which is exactly why a mismatch is a non-blocking prompt to verify rather than a hard failure. It is skipped for any plugin that declares neither array.

## The scaffolder (`lib/scaffold`)

It derives a prefix set (pascal / kebab / snake / upper forms) from the vendor and integration names and rewrites the example tokens (`ExampleVendor`, `example-integration`, `VIP_EXAMPLE_INTEGRATION`, ...) across the tree. Replacement uses PHP `strtr` semantics — longest match wins and a replacement is never re-scanned.

## Dependencies

Runtime: [`commander`](https://github.com/tj/commander.js) for argument parsing, [`js-yaml`](https://github.com/nodeca/js-yaml) to parse the handoff manifest, and [`ajv`](https://ajv.js.org/) to validate it against the manifest JSON Schema. Colors are a ~15-line ANSI helper rather than a dependency, which keeps the build a plain CommonJS `tsc` compile with no ESM-only packages. Dev: `typescript`, `jest`, and `ts-jest` for the build and tests, plus `eslint` with [`@automattic/eslint-plugin-wpvip`](https://github.com/Automattic/eslint-plugin-wpvip) and `wp-prettier` for lint/format — the same tooling as [Automattic/commands](https://github.com/Automattic/commands). The package manager is **pnpm** (pinned via `packageManager`).
Runtime: [`commander`](https://github.com/tj/commander.js) for argument parsing, [`js-yaml`](https://github.com/nodeca/js-yaml) to parse the handoff manifest, and [`ajv`](https://ajv.js.org/) to validate it against the manifest JSON Schema. Colors are a ~20-line ANSI helper rather than a dependency, which keeps the build a plain CommonJS `tsc` compile with no ESM-only packages. Dev: `typescript`, `jest`, and `ts-jest` for the build and tests, plus `eslint` with [`@automattic/eslint-plugin-wpvip`](https://github.com/Automattic/eslint-plugin-wpvip) and `wp-prettier` for lint/format — the same tooling as [Automattic/commands](https://github.com/Automattic/commands). The package manager is **pnpm** (pinned via `packageManager`).
12 changes: 12 additions & 0 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ export interface ValidateOptions {
}

export function validateCommand( pathArg: string | undefined, opts: ValidateOptions = {} ): void {
// An explicit but empty path (`validate ""`, or an unset shell variable) is a
// mistake, not a request to validate the current directory — reject it rather
// than silently checking cwd. An omitted argument still defaults to cwd.
if ( pathArg !== undefined && pathArg.trim() === '' ) {
console.error(
red(
'No path given. Pass an integration directory, or omit it to use the current directory.'
)
);
process.exitCode = 1;
return;
}
const root = resolve( pathArg ?? process.cwd() );

const format = opts.format ?? 'human';
Expand Down
6 changes: 5 additions & 1 deletion src/lib/validate/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ export function inspectManifest( root: string ): ManifestInspection {

let parsed: unknown;
try {
parsed = load( readFileSync( join( root, file ), 'utf8' ) );
// Reject YAML anchors/aliases outright. A manifest never needs them, and
// nested aliases are a billion-laughs vector: js-yaml returns them as shared
// references cheaply, but the downstream walks (placeholder scan, Ajv) expand
// them into an exponential tree and hang. Failing at parse time closes it.
parsed = load( readFileSync( join( root, file ), 'utf8' ), { maxAliases: 0 } );
} catch ( error ) {
const reason = error instanceof Error ? error.message.split( '\n' )[ 0 ] : String( error );
return { file, parseError: reason, errors: [], parsed: null, placeholders: [] };
Expand Down
85 changes: 70 additions & 15 deletions src/lib/validate/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,62 @@ function claimsCompatibilityException( composer: ComposerJson | null ): boolean
return ( vip as Record< string, unknown > )[ 'compatibility-exception' ] === 'approved';
}

/**
* The version tokens in a matrix key's same-line value. A flow array (`[…]`) is
* read to its closing bracket; any other value stops at the next flow-mapping
* entry boundary (`,` / `}`) or an inline `#` comment — so `wp: 6.9, php: '8.5'`
* doesn't bleed the PHP value into the WordPress scan, and a trailing comment's
* versions aren't mistaken for coverage.
*/
function valueTokens( rawValue: string, tokenRe: RegExp ): RegExpMatchArray | null {
const value = rawValue.trim();
if ( value.startsWith( '[' ) ) {
const end = value.indexOf( ']' );
return ( end === -1 ? value : value.slice( 1, end ) ).match( tokenRe );
}
return value.split( /[,}#]/ )[ 0 ].match( tokenRe );
}

/**
* Collect the version tokens a CI matrix lists against a given key (`php`, `wp`,
* …), scoped so unrelated tokens elsewhere in the workflow aren't counted — a
* bare `mysql:8.4` or a `node: [6.9, 7.0]` matrix must not read as PHP or
* WordPress coverage. Handles the three common GitHub Actions matrix forms: a
* same-line scalar (`php: '8.5'`), a flow array (`php: [8.2, 8.3]`), and a block
* sequence (`php:` then `- '8.2'` items on the next lines). Returns `x.y`
* version numbers plus the literal `latest`.
*/
function collectMatrixVersions( workflowsText: string, keyPattern: string ): Set< string > {
const versions = new Set< string >();
const lines = workflowsText.split( /\r?\n/ );
// keyPattern is a fixed internal literal, so interpolation is safe.
// eslint-disable-next-line security/detect-non-literal-regexp
const keyRe = new RegExp( String.raw`\b(?:${ keyPattern })['"]?\s*[:=]\s*([^\n]*)`, 'gi' );
const tokenRe = /\d+\.\d+|latest/gi;
const itemRe = /^\s*-\s*['"]?(\d+\.\d+|latest)/i;

for ( let line = 0; line < lines.length; line++ ) {
for ( const key of lines[ line ].matchAll( keyRe ) ) {
for ( const token of valueTokens( key[ 1 ], tokenRe ) ?? [] ) {
versions.add( token.toLowerCase() );
}
if ( key[ 1 ].trim() !== '' ) {
continue;
}
// Nothing follows the key: a block sequence carries the values on the
// next lines as `- 8.2` items. Read them until the sequence ends.
for ( let next = line + 1; next < lines.length; next++ ) {
const item = itemRe.exec( lines[ next ] );
if ( ! item ) {
break;
}
versions.add( item[ 1 ].toLowerCase() );
}
}
}
return versions;
}

function checkCompatibilityMatrix( ctx: Context ): CheckResult {
const base = {
id: 'compatibility-matrix',
Expand Down Expand Up @@ -844,25 +900,24 @@ function checkCompatibilityMatrix( ctx: Context ): CheckResult {
}

const missing: string[] = [];
if ( ! /\b6\.9\b/.test( ctx.workflowsText ) ) {
// Scope the WordPress scan to a `wp` / `wordpress` matrix key, exactly as PHP
// is scoped below — a bare `6.9`/`7.0` elsewhere (a `node` matrix, an action
// tag, `runs-on: ubuntu-latest`) is not WordPress evidence. WP 7.0 in CI is
// often written as `wp: latest`, so that counts too.
const wpVersions = collectMatrixVersions(
ctx.workflowsText,
'w(?:p|ordpress)(?:[-_]versions?)?'
);
if ( ! wpVersions.has( '6.9' ) ) {
missing.push( 'WordPress 6.9' );
}
// WP 7.0 in CI is often expressed as "latest" against a WordPress version
// key (e.g. `wp: latest`). Match that form specifically so unrelated
// `latest` tokens — `runs-on: ubuntu-latest`, `mariadb:latest` — are not
// mistaken for WordPress version evidence.
const wpLatest = /\bw(?:p|ordpress)(?:[-_]version)?['":= ]+latest\b/i;
if ( ! /\b7\.0\b/.test( ctx.workflowsText ) && ! wpLatest.test( ctx.workflowsText ) ) {
if ( ! wpVersions.has( '7.0' ) && ! wpVersions.has( 'latest' ) ) {
missing.push( 'WordPress 7.0' );
}
// Only count a PHP version that sits against a `php` / `php-version` key. A
// bare `.includes('8.4')` matches mysql:8.4, a node 18.4 matrix, or a pinned
// action tag, so an integration testing only 8.2 could report full coverage.
const phpVersions = new Set(
[ ...ctx.workflowsText.matchAll( /php(?:[-_]version)?['":= ]+['"]?(\d+\.\d+)/gi ) ].map(
match => match[ 1 ]
)
);
// Accept `php-versions` (plural) alongside `php` / `php-version` — the plural
// is the key `shivammathur/setup-php` examples use, so a conformant matrix
// must not be failed just for pluralizing it.
const phpVersions = collectMatrixVersions( ctx.workflowsText, 'php(?:[-_]versions?)?' );
for ( const php of [ '8.2', '8.3', '8.4', '8.5' ] ) {
if ( ! phpVersions.has( php ) ) {
missing.push( `PHP ${ php }` );
Expand Down
Loading