Skip to content

Add vip integration validate conformance checker#2978

Closed
aswasif007 wants to merge 6 commits into
feature/vip-integrationfrom
add/validate-integration-command
Closed

Add vip integration validate conformance checker#2978
aswasif007 wants to merge 6 commits into
feature/vip-integrationfrom
add/validate-integration-command

Conversation

@aswasif007

@aswasif007 aswasif007 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Adds vip integration validate, a no-auth command that checks a WordPress VIP partner integration for conformance and returns an objective conformant / not-conformant verdict. A partner (or an internal team) will run it locally and in CI before submitting an integration.

  • Runs entirely on local files (composer.json, package.json, docs, PHP source, CI workflows).
  • Nine automated rules, each mapped to a conformance-checklist item and reporting which rule failed and why. --format json for machine-readable CI output; exits non-zero when an integration is not conformant.
  • The two items that we don't want to automate - security review and the plugin/platform config-schema match - are printed as a "Human review required" section.

See docs/integration-validate.md for the command reference and the full rule list.

Changelog Description

Added

  • Added vip integration validate, a conformance checker for WordPress VIP partner integrations that runs locally and in CI and returns an objective conformant / not-conformant verdict.

Pull request checklist

New release checklist

Steps to Test

  1. Check out the PR branch and run npm run build.
  2. Enter ./dist/bin/vip.js integration validate and make sure that it doesn't require auth.
  3. Run ./dist/bin/vip.js integration validate <path-to-an-integration> against a conformant integration and see the output.
    Tip💡: You can locally pull the update/integration-contract-compliance branch of the vip-integrations-starter-kit repo, and run the validator against it: ./dist/bin/vip.js integration validate ~/automattic/vip-integrations-starter-kit.
Sample Success Result
Integration conformance check: /Users/aswasif007/automattic/vip-integrations-starter-kit

  PASS  Rule 1: Loads through the Starter Kit workflow
        Plugin entry file and Composer wordpress-plugin package are present.
        - Entry file: example-integration.php
        - composer.json type is "wordpress-plugin" with an autoload section.
  PASS  Rule 2: `composer test` runs PHPUnit and e2e tests
        composer test declares a PHPUnit run and an e2e runner (Playwright/Cypress).
        - Resolved commands: phpunit • npm test • playwright test
        - Static check: it verifies the test commands are wired, not that the tests pass.
  PASS  Rule 3: `composer run validate-integration` exists
        composer.json defines a "validate-integration" script.
  PASS  Rule 4: Config constant is documented and referenced in code
        Config constant VIP_EXAMPLE_INTEGRATION_CONFIG is referenced in code and documented.
  PASS  Rule 5: Missing/invalid config is handled without fataling
        Config access is guarded against a missing or invalid constant.
        - Guards found: is_ready(), missing_fields(), is_available(), defined() guard, is_array() guard
        - Static signal only — behavioral proof comes from the integration's own tests (rule 2).
  PASS  Rule 6: Docs include valid and incomplete config examples
        Docs include both a valid and an incomplete config example.
  PASS  Rule 7: Compatibility evidence covers WP 6.9/7.0 and PHP 8.2-8.5
        CI matrix covers WP 6.9 + 7.0 and PHP 8.2-8.5.
  PASS  Rule 8: Build and test commands are documented
        Docs document both build/install and test commands.
  PASS  Rule 9: Telemetry uses the Starter Kit pattern (Tracks only, no secrets)
        Telemetry uses the guarded VIP Tracks helper with no obvious secrets in properties.

Human review required (not automated):
  • Plugin - platform config-schema match
    Whether the plugin's expected config matches the platform schema is not fully deterministic and is confirmed in human review, not by this checker.
  • Security review
    Security posture (input handling, secret storage, capability checks) is assessed in human review, not by this checker.

Summary: 9 passed, 0 failed, 0 warnings, 0 n/a.
✓ Conformant — no automated checks failed.
  1. Run it against a non-conformant integration (e.g. one whose composer.json has no test script): confirm the relevant rule reports FAIL and the process exits non-zero (echo $?1).
    Tip💡: Now checkout the main branch of the vip-integrations-starter-kit repo, and run the same command again.
Sample Failure Result
Integration conformance check: /Users/aswasif007/automattic/vip-integrations-starter-kit

  PASS  Rule 1: Loads through the Starter Kit workflow
        Plugin entry file and Composer wordpress-plugin package are present.
        - Entry file: index.php
        - composer.json type is "wordpress-plugin" with an autoload section.
  FAIL  Rule 2: `composer test` runs PHPUnit and e2e tests
        composer test does not wire up: an e2e runner (no Playwright/Cypress invocation).
        - Resolved commands: phpunit
  FAIL  Rule 3: `composer run validate-integration` exists
        composer.json has no "validate-integration" script.
  N/A   Rule 4: Config constant is documented and referenced in code
        No runtime config constant detected (looked for a `Config::CONSTANT_NAME` declaration or a `VIP_*_CONFIG` constant). If this integration uses runtime config under another name, adopt the convention or flag it for human review — these config checks were skipped, not passed.
  N/A   Rule 5: Missing/invalid config is handled without fataling
        No runtime config constant detected (looked for a `Config::CONSTANT_NAME` declaration or a `VIP_*_CONFIG` constant). If this integration uses runtime config under another name, adopt the convention or flag it for human review — these config checks were skipped, not passed.
  N/A   Rule 6: Docs include valid and incomplete config examples
        No runtime config constant detected (looked for a `Config::CONSTANT_NAME` declaration or a `VIP_*_CONFIG` constant). If this integration uses runtime config under another name, adopt the convention or flag it for human review — these config checks were skipped, not passed.
  FAIL  Rule 7: Compatibility evidence covers WP 6.9/7.0 and PHP 8.2-8.5
        CI compatibility matrix is missing: WordPress 6.9, PHP 8.4, PHP 8.5.
        - Cover the matrix in CI (.github/workflows) or add an approved compatibility exception note.
  FAIL  Rule 8: Build and test commands are documented
        Docs are missing: build/install commands.
  N/A   Rule 9: Telemetry uses the Starter Kit pattern (Tracks only, no secrets)
        The integration does not record telemetry.

Human review required (not automated):
  • Plugin - platform config-schema match
    Whether the plugin's expected config matches the platform schema is not fully deterministic and is confirmed in human review, not by this checker.
  • Security review
    Security posture (input handling, secret storage, capability checks) is assessed in human review, not by this checker.

Summary: 1 passed, 4 failed, 0 warnings, 4 n/a.
✗ Not conformant — one or more automated checks failed.

@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Comment thread docs/validate-integration.md Outdated
Comment thread docs/validate-integration.md Outdated
Comment thread src/lib/validate-integration/validate.ts Outdated
@aswasif007 aswasif007 changed the title Add/validate integration command Add vip validate integration conformance checker Jul 15, 2026
@aswasif007
aswasif007 force-pushed the add/validate-integration-command branch from 1e0306a to ff3ad26 Compare July 15, 2026 09:56
@aswasif007
aswasif007 marked this pull request as ready for review July 15, 2026 10:12
@aswasif007
aswasif007 requested review from andrea-sdl and pandah3 July 15, 2026 10:12
Comment thread src/lib/cli/internal-bin-loader.js Outdated
Comment on lines +67 to +68
'vip-validate': () => import( '../../bin/vip-validate' ),
'vip-validate-integration': () => import( '../../bin/vip-validate-integration' ),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would switch the order of the params. vip integration <command> since we might need init/configure in the future.

I also wonder if this should belong to the VIP-CLI or if we should have a dedicate tooling for the integrations, but that's probably ok to keep it here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wonder if this should belong to the VIP-CLI or if we should have a dedicate tooling for the integrations, but that's probably ok to keep it here.

I don't think it should be here TBH.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should be here TBH.

@rinatkhaziev like a separate CLI command, correct?

vip dev-env would still be something used (and useful) for a partner to test an integration, so it could make sense to bundle it here, instead of separating it in a separate CLI.

On the other hand, separating it would probably give us a little bit more freedom, but we'd consolidate less on the pre-existing codebase.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrea-sdl I've flipped the command.

Regarding whether we should put the command here or create a separate package: both options have pros and cons. Let's discuss this, but we also have to keep moving with the SDK integration. So I will release this command with next tag instead of merging it to trunk. If we decide in the meantime that this command don't belong here, we will port the code away elsewhere.

Static checks that map the VIP integration conformance checklist to an
objective pass/fail/warn/n-a verdict per rule, plus human-readable and
JSON report rendering. Security review and the plugin/platform
config-schema match stay in the human-review layer, never an automated
pass.
Register the no-auth integration command and its validate subcommand,
wire the bin entries and internal-bin loader, and gate CI with a
non-zero exit when an integration is not conformant.
Cover conformant and non-conformant integrations, the load/test/config
edge cases, and the report rendering.
@aswasif007
aswasif007 force-pushed the add/validate-integration-command branch from ff3ad26 to f87c5e9 Compare July 16, 2026 06:40
@aswasif007 aswasif007 changed the title Add vip validate integration conformance checker Add vip integration validate conformance checker Jul 16, 2026
@aswasif007
aswasif007 changed the base branch from trunk to feature/vip-integration July 16, 2026 11:23
@aswasif007

aswasif007 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

@andrea-sdl @pandah3 @rinatkhaziev Unless ya'll don't have an objection, I will proceed to merge it into feature/vip-integration branch, from which I will release version 4.2.0-dev.0. I will also be adding vip integration init command in the same version. After that we will be able to test/simulate things from the partner-developers' standpoint.

Comment thread src/bin/vip.js Outdated
Comment thread src/lib/validate-integration/validate.ts Outdated

@andrea-sdl andrea-sdl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested using the starter kit.

The only thing worth mentioning (which is more related to the starter kit) is that once I run composer setup -- --vendor="Acme" --name="Content Sync" in the starter kit, we fail the validation

Besides this, it looks good for me. I left some feedback inline, the only relevant is the latest behavior that might generate false positives.

Let's check in with @rinatkhaziev on merging it here vs in a separate repo.

node dist/bin/vip.js integration validate
Integration conformance check: vip-integrations-starter-kit

  PASS  Rule 1: Loads through the Starter Kit workflow
        Plugin entry file and Composer wordpress-plugin package are present.
        - Entry file: content-sync.php
        - composer.json type is "wordpress-plugin" with an autoload section.
  PASS  Rule 2: `composer test` runs PHPUnit and e2e tests
        composer test declares a PHPUnit run and an e2e runner (Playwright/Cypress).
        - Resolved commands: phpunit • npm test • playwright test
        - Static check: it verifies the test commands are wired, not that the tests pass.
  PASS  Rule 3: `composer run validate-integration` exists
        composer.json defines a "validate-integration" script.
  FAIL  Rule 4: Config constant is documented and referenced in code
        Config constant VIP_CONTENT_SYNC_CONFIG is used in code but not documented in README/docs.
  PASS  Rule 5: Missing/invalid config is handled without fataling
        Config access is guarded against a missing or invalid constant.
        - Guards found: is_ready(), missing_fields(), is_available(), defined() guard, is_array() guard
        - Static signal only — behavioral proof comes from the integration's own tests (rule 2).
  FAIL  Rule 6: Docs include valid and incomplete config examples
        No documented config example references VIP_CONTENT_SYNC_CONFIG.
  PASS  Rule 7: Compatibility evidence covers WP 6.9/7.0 and PHP 8.2-8.5
        CI matrix covers WP 6.9 + 7.0 and PHP 8.2-8.5.
  PASS  Rule 8: Build and test commands are documented
        Docs document both build/install and test commands.
  PASS  Rule 9: Telemetry uses the Starter Kit pattern (Tracks only, no secrets)
        Telemetry uses the guarded VIP Tracks helper with no obvious secrets in properties.

Human review required (not automated):
  • Plugin - platform config-schema match
    Whether the plugin's expected config matches the platform schema is not fully deterministic and is confirmed in human review, not by this checker.
  • Security review
    Security posture (input handling, secret storage, capability checks) is assessed in human review, not by this checker.

Summary: 7 passed, 2 failed, 0 warnings, 0 n/a.
✗ Not conformant — one or more automated checks failed.

Skipping auth on any argv containing "integration" let a logged-out
call like `vip @app.env -- wp option get integration` bypass the login
prompt and fail. Gate the no-auth path on the absence of an app/env
argument, mirroring dev-env.
The rule 7 check treated any `latest` token as WP 7.0 evidence, so a
`runs-on: ubuntu-latest` runner label wrongly satisfied it. Match
`latest` only when tied to a WordPress version key (e.g. `wp: latest`).
@sonarqubecloud

Copy link
Copy Markdown

@aswasif007

aswasif007 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

The validate command fails after composer setup because the VIP_....._CONFIG is not documented yet. I am not sure if failing is an incorrect behaviour, because the user has to open the doc and update it.

But on another thought, the validate command shouldn't fail right after the developer sets the stage for work by running the setup. 🤔

In any case, the fix will be in the starter kit repo. Not here.

@sjinks

sjinks commented Jul 19, 2026

Copy link
Copy Markdown
Member

Adversarial Review: vip integration validate

Pull request: Automattic/vip-cli#2978
Branch: add/validate-integration-command
Review status: Not ready to approve

Summary

This pull request adds vip integration validate, a static conformance checker
for WordPress VIP partner integrations. It adds human-readable and JSON reports,
validates Composer configuration, test wiring, documentation, compatibility
coverage, and telemetry patterns, and makes the command available without login.

The command is presented as a CI-ready conformance gate. Four issues prevent that
claim from being reliable: two checks can be satisfied with non-executing text,
the file scanner can escape the target directory through symbolic links, and the
authentication bypass is triggered by any argument named integration.

Findings

P1: Rule 2 accepts printed test-runner names as test execution

Location: src/lib/validate-integration/validate.ts, checkComposerTest

The checker concatenates the resolved Composer commands and uses token matching
to determine whether PHPUnit and an e2e runner are present. It does not establish
that either command executes.

For example, this script exits successfully without running tests, but receives a
passing Rule 2 result:

{
	"scripts": {
		"test": "false || printf phpunit-playwright"
	}
}

The printf output contains both required tokens, so the check reports that
composer test wires up PHPUnit and e2e coverage. This enables a green
conformance report where no tests are actually run.

Recommendation: Parse the Composer script structure sufficiently to identify
actual command invocations, or classify this evidence as warn rather than
allowing it to satisfy a conformance gate. Add a regression fixture for the
example above.

P1: Rule 7 accepts comments as a CI compatibility matrix

Location: src/lib/validate-integration/validate.ts, checkCompatibilityMatrix

The checker treats any nonempty workflow file as CI evidence and searches its raw
text for the required WordPress and PHP version strings. A workflow containing
only this comment passes Rule 7 despite defining no jobs or matrix:

# WordPress 6.9 and 7.0; PHP 8.2, 8.3, 8.4, 8.5
name: empty

Combined with the Rule 2 bypass, I reproduced a report with conformant: true
for an integration that has neither executable tests nor a CI matrix.

Recommendation: Parse workflow YAML and inspect a relevant job matrix. At a
minimum, strip comments and require version values to appear in a workflow job or
matrix. Use warn when the static evidence cannot be structurally verified.

P2: File scanning follows symbolic links outside the integration root

Location: src/lib/validate-integration/validate.ts, collectFiles

The scanner uses statSync(), which follows symbolic links, then reads every
matching source, documentation, and workflow file into memory. A repository can
therefore link outside the selected integration directory. A link to a large tree
or a special file such as /dev/zero with a matching extension can make the CLI
scan external files, hang, or exhaust memory.

Recommendation: Use lstatSync() to skip symbolic links, accept only regular
files, enforce realpath containment below the selected root, and apply a
reasonable per-file byte limit.

P2: Authentication is bypassed when any argument equals integration

Location: src/bin/vip.js, isIntegrationCommand

The login exemption uses argv.some(...), rather than determining the selected
top-level command. An unauthenticated invocation such as either of the following
skips the normal login flow even though it is not the local integration command:

vip app list integration
vip wp integration

The downstream API command is still likely to fail without credentials, but the
root command's authentication behavior has regressed.

Recommendation: Derive the exemption from the parsed top-level command token
before --, not from an arbitrary positional argument. Add command-level tests
for both the intended unauthenticated invocation and unrelated commands that use
integration as an argument.

Validation Performed

  • npx jest __tests__/lib/validate-integration.test.ts __tests__/lib/validate-integration-report.test.ts --runInBand
    • Passed: 2 suites, 15 tests.
  • NODE_ENV=test npm run jest -- validate-integration
    • Passed: 72 suites, 698 tests. The package-script forwarding ran the full
      Jest suite rather than only the intended validator tests.
  • NODE_ENV=test npm run check-types
    • Passed.

Approval Criteria

Do not approve until the two conformance-gate bypasses and the authentication
dispatch regression are fixed. The filesystem traversal should also be bounded
before encouraging users to run this tool against untrusted repositories or in
shared CI environments.

@aswasif007

aswasif007 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR in favour of Automattic/integration#1

@aswasif007 aswasif007 closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants