Fix 1Password writes failing with "invalid JSON in piped input" by ahegyes · Pull Request #135 · a8cteam51/team51-cli

ahegyes · 2026-06-30T14:07:42Z

Problem

When cloning or creating a Pressable/WPCOM site, the CLI rotates the WP admin password and tries to save it to 1Password. The save fails and the new password is printed to the terminal in plaintext instead of being stored. 1Password reads (SSH/SFTP credential lookups) keep working — only writes break.

❌ 1Password CLI failed (exit 1): [ERROR] ... invalid JSON in piped input
1Password login entry could not be created.

Root cause

op item create / op item edit read a piped stdin as a JSON item template and abort with invalid JSON in piped input when it isn't one. Commit a796de4 routed every op call through Symfony Process, which always wires the child's stdin to a pipe — so every 1Password write since has hit the template path and failed mid-provision. Reads were unaffected because op item list / op item get ignore stdin.

Fix

Build each op command as an argument vector and run it through a minimal sh -c 'exec "$@" < /dev/null' wrapper:

op's stdin is /dev/null, so it stays on the argument-based path.
Passing argv as an array (not a shell string) also removes the unescaped '$field=$value' interpolation and Symfony's "${:VAR}" placeholder substitution.
Item IDs are passed as argv elements rather than interpolated into a shell string.
Symfony Process is retained, so the 60s timeout and transient-retry loop are preserved.

Additional hardening (surfaced during review)

The package-bump commit on this branch exposed some adjacent issues, addressed here:

PHP 8.3 dependency floor. That bump pulled symfony/string / symfony/var-exporter v8.1 (PHP 8.4-only) into the lock, breaking a fresh composer install on the declared 8.3 target. Re-pinned to the 7.4 line; narrowed the packages-update script to --ignore-platform-req=ext-* so future updates respect the floor; declared require.php: ">=8.3".
Install resilience. composer install and dump-autoload in install-osx / self-update.php now pass --ignore-platform-reqs, so a platform/PHP mismatch no longer hard-aborts install/self-update.
Startup environment guard. New environment-guard.php (shared by team51-cli.php and mcp-server.php, before autoload) checks PHP ≥ 8.3 and the required extensions, failing with a clear message instead of a cryptic fatal.
Docs & formatting. README aligned to PHP 8.3; phpcbf (WPCS) formatting; a (string) cast restoring the WPCOM rotate skip-guard after phpcbf changed == to ===; the MCP github_set_topics example kept lowercase under phpcbf via @phpcs:ignore.

Testing

Reproduced the original failure (empty stdin pipe → invalid JSON in piped input) and proved /dev/null fixes it.
All six op helpers verified to emit correct argv with /dev/null stdin; field values containing ', ", $, backticks, $(…), and "${:VAR}" round-trip exactly with no injection or substitution; real op item create --dry-run confirmed end-to-end.
packages-update floor proven by A/B dry-run (old flag re-pulls symfony 8.x; new flag holds at 7.4).
Startup guard exercised; php -l and phpcs clean on touched files; composer install --dry-run reports nothing to install with zero >=8.4 requirements.

Fixes #134
Linear: T51ENG-1955

`op item create`/`edit` read a piped stdin as a JSON item template and abort with "invalid JSON in piped input" when it isn't one. Commit a796de4 routed op through Symfony Process, which always wires the child's stdin to a pipe, so every 1Password write since has failed mid-provision and printed the rotated password to the terminal in plaintext. Reads were unaffected because `op item list`/`get` ignore stdin. Build the op command as an argument vector and run it through a minimal `sh -c 'exec "$@" < /dev/null'` wrapper, so op's stdin is /dev/null and it stays on the argument-based path. Passing argv as an array (rather than a shell string) also removes the unescaped `'$field=$value'` interpolation and Symfony's `"${:VAR}"` placeholder substitution, either of which could corrupt or crash a write whose value held shell- or Symfony-special characters. The Symfony Process 60s timeout and transient-retry loop are preserved. Refs: T51ENG-1955, #134 Assisted-by: Claude Code:claude-opus-4-8

Whitespace/standards changes from `composer run format:php` (phpcbf): array `=>`/`=` alignment, union-type spacing, docblock "WordPress" capitalization, and a loose-to-strict (`==` -> `===`) comparison in WPCOM_Site_WP_User_Password_Rotate.php. Assisted-by: Claude Code:claude-opus-4-8

phpcbf's loose-to-strict change made `'242557543' === $site->ID` compare a string literal against the integer ID the WPCOM API returns, so the guard that skips the known-broken site stopped matching. Cast the ID to string to keep the strict comparison while restoring the match. Assisted-by: Claude Code:claude-opus-4-8

The earlier package update ran `composer update --ignore-platform-reqs`, which pulled symfony/string and symfony/var-exporter v8.1 (php >=8.4.1) into the lock while the project targets PHP 8.3 — so a fresh `composer install` (install-osx, self-update.php) failed the platform check. Re-resolve with the 8.3 floor enforced and only ext-* ignored, pinning both packages back to the 7.4 line. Assisted-by: Claude Code:claude-opus-4-8

GitHub repository topics are lowercase. phpcbf's CapitalPDangit had auto-capitalized the github_set_topics example to "WordPress", which the MCP tool schema surfaces to clients as a suggested (invalid) topic. Assisted-by: Claude Code:claude-opus-4-8

install-osx and self-update.php ran a plain `composer install`, so a lock entry whose php/extension requirement isn't satisfied aborts the whole install/self-update. Pass --ignore-platform-reqs (as the packages-install script already does) so installation no longer hard-exits on a platform mismatch, leaving the CLI free to handle such cases itself. Assisted-by: Claude Code:claude-opus-4-8

@phpcs

GitHub topics are lowercase, but WordPress.WP.CapitalPDangit auto-fixes the github_set_topics example to "WordPress" on `format:php`. Add a @phpcs:ignore docblock tag — placed as a tag so it stays out of the generated MCP tool schema description — to keep the example lowercase. Assisted-by: Claude Code:claude-opus-4-8

Check the PHP version (>= 8.3) and required extensions (gd, json, posix, readline) before autoload, and exit with a clear message if any are unmet, so an unsupported runtime fails up front instead of as a cryptic fatal inside a dependency loaded under --ignore-platform-reqs. Assisted-by: Claude Code:claude-opus-4-8

packages-update ran `composer update --ignore-platform-reqs`, which ignores the config.platform php pin and pulls packages requiring a newer PHP — the symfony 8.1 / php-8.4 bump that broke a fresh install. Narrow it to --ignore-platform-req=ext-* so updates still ignore missing extensions but honor the 8.3 version floor. (packages-install keeps the lenient flag: it installs the existing lock and shouldn't hard-exit on a platform mismatch.) Assisted-by: Claude Code:claude-opus-4-8

The startup guard lived inline in team51-cli.php, so the documented direct entry point `php mcp-server.php` skipped it and could still hit a cryptic fatal after an --ignore-platform-reqs install. Extract it to environment-guard.php and require it (before autoload) from both entry points. Assisted-by: Claude Code:claude-opus-4-8

The README still told users PHP 8.2 was fine and composer.json declared no php floor at all, while the runtime guard and config.platform require 8.3. Bump the README to 8.3 and add "php": ">=8.3" to composer.json require (lock content-hash resynced). Assisted-by: Claude Code:claude-opus-4-8

`composer install` was relaxed with --ignore-platform-reqs but the following `composer dump-autoload -o` was not, so it could still abort on a platform mismatch and bake a strict vendor/composer/platform_check.php. Pass the flag to the dump step as well, in install-osx and self-update.php. Assisted-by: Claude Code:claude-opus-4-8

t51eng-poseidon

🔱 Poseidon Review

Summary — Fixes 1Password writes that broke when every op call was routed through a Symfony pipe by running op through an sh wrapper with stdin at /dev/null and passing argv as an array; low risk, well-scoped, and the argv change also removes the prior shell-quoting/injection surface. One non-blocking note on the new environment guard.

🐛 Fix — op item create/edit no longer hit the JSON-template stdin path; writes succeed again instead of leaking the rotated password to the terminal.
♻️ Refactor — 1Password helpers build an argv vector (_build_1password_command_args) instead of a shell string, dropping escapeshellarg/Process::fromShellCommandline.
🔧 Chore — PHP floor pinned to >=8.3, symfony re-pinned to the 7.4 line, install/self-update pass --ignore-platform-reqs, new pre-autoload environment-guard.php, WPCS formatting.
📝 Docs — README aligned to PHP 8.3.

environment-guard.php:L14 — The guard only enforces the project's own floor (PHP_VERSION_ID < 80300), but adding --ignore-platform-reqs to composer dump-autoload in install-osx/self-update.php suppresses composer's generated platform_check.php, which normally rejects a PHP older than the strictest installed dependency requires. So the scenario the docblock cites — "a package installed under --ignore-platform-reqs that needs a newer PHP" — would still pass this guard (8.3 >= 8.3) and fatal at runtime if any dependency ever required >=8.4. It is latent today because symfony is re-pinned to the 7.4 line, but consider softening the docblock claim or having the guard reflect the true dependency floor so the comment and behavior match.

🤖 Prompt for AI agents — fix all findings

Verify each finding against the current code and only fix it if
needed.

Suggestions:
In `environment-guard.php`:
- Line 14: The guard's version check `if ( PHP_VERSION_ID <
  80300 )` enforces only the project's own >=8.3 floor, while
  the docblock above claims it guards against "a package
  installed under --ignore-platform-reqs that needs a newer
  PHP". Because `install-osx` and `self-update.php` now run
  `composer dump-autoload -o --ignore-platform-reqs`, composer
  no longer emits `platform_check.php`, so this guard is the
  only runtime gate and it will pass on PHP 8.3 even if an
  installed dependency requires >=8.4, causing a later fatal.
  Either soften the docblock to state the guard checks only the
  project floor, or compute/hardcode the strictest installed
  PHP requirement in the version check so the comment and
  behavior agree; keep the extension-loaded loop and the STDERR
  messaging unchanged. This is latent while symfony stays on
  the 7.4 line, so treat it as non-blocking.

ℹ️ Review info

Commit: c1a72bf
Files reviewed: README.md, commands/GitHub_Checklist_Add.php, commands/WPCOM_Site_WP_User_Password_Rotate.php, composer.json, environment-guard.php, includes/functions-1password.php, includes/functions-github.php, install-osx, mcp-server.php, mcp/Team51McpTools.php, self-update.php, team51-cli.php
Files skipped: composer.lock (generated lockfile); the bulk of mcp/Team51McpTools.php, commands/GitHub_Checklist_Add.php, and includes/functions-github.php changes are phpcbf whitespace/alignment only (verified, no logic change)
Model: claude-opus-4-8

t51eng-poseidon · 2026-07-01T11:34:00Z

+( static function (): void {
+	$problems = array();
+
+	if ( PHP_VERSION_ID < 80300 ) {


The guard enforces only the project's own floor (PHP_VERSION_ID < 80300), but adding --ignore-platform-reqs to composer dump-autoload in install-osx/self-update.php suppresses composer's generated platform_check.php — the file that normally rejects a PHP older than the strictest installed dependency requires. As a result the scenario this file's docblock cites ("a package installed under --ignore-platform-reqs that needs a newer PHP") would still pass here (8.3 >= 8.3) and fatal at runtime if any dependency ever required >=8.4. It is latent today because symfony is re-pinned to the 7.4 line; consider softening the docblock claim or reflecting the true dependency floor so comment and behavior match.

🤖 Fix prompt for AI agents

Verify each finding against the current code and only fix it if needed. In `environment-guard.php`: - Line 14: The guard's version check `if ( PHP_VERSION_ID < 80300 )` enforces only the project's own >=8.3 floor, while the docblock above claims it guards against "a package installed under --ignore-platform-reqs that needs a newer PHP". Because `install-osx` and `self-update.php` now run `composer dump-autoload -o --ignore-platform-reqs`, composer no longer emits `platform_check.php`, so this guard is the only runtime gate and it will pass on PHP 8.3 even if an installed dependency requires >=8.4, causing a later fatal. Either soften the docblock to state the guard checks only the project floor, or compute/hardcode the strictest installed PHP requirement in the version check so the comment and behavior agree; keep the extension-loaded loop and the STDERR messaging unchanged. This is latent while symfony stays on the 7.4 line, so treat it as non-blocking.

t51eng-poseidon · 2026-07-01T12:18:02Z

+( static function (): void {
+	$problems = array();
+
+	if ( PHP_VERSION_ID < 80300 ) {


The guard's version check if ( PHP_VERSION_ID < 80300 ) enforces only the project's own >=8.3 floor, yet the docblock above it claims it guards against "a package installed under --ignore-platform-reqs that needs a newer PHP." Because install-osx and self-update.php now run composer dump-autoload -o --ignore-platform-reqs, composer no longer emits its generated platform_check.php, so this guard becomes the only runtime gate — and it will pass on PHP 8.3 even if an installed dependency requires >=8.4, then fatal deep in that dependency. It's latent today only because symfony is re-pinned to the 7.4 line. Either soften the docblock to say the guard checks just the project floor, or reflect the true strictest-dependency PHP requirement in the check so comment and behavior agree.

🤖 Fix prompt for AI agents

Verify each finding against the current code and only fix it if needed. In `environment-guard.php`: - Line 14: The version check `if ( PHP_VERSION_ID < 80300 )` enforces only the project's own >=8.3 floor, while the docblock above claims the guard protects against "a package installed under --ignore-platform-reqs that needs a newer PHP". Because `install-osx` and `self-update.php` now run `composer dump-autoload -o --ignore-platform-reqs`, composer no longer emits `platform_check.php`, so this guard is the only runtime gate and it passes on PHP 8.3 even when an installed dependency requires >=8.4, causing a later fatal deep in that dependency. Either soften the docblock to state the guard checks only the project floor, or compute/hardcode the strictest installed PHP requirement in the version check so the comment and behavior agree; keep the extension-loaded foreach loop and the STDERR $problems messaging unchanged. This is latent while symfony stays on the 7.4 line, so treat it as non-blocking.

t51eng-poseidon

🔱 Poseidon Review

Summary — Fixes 1Password writes that broke when every op call was routed through a Symfony pipe, by running op through an sh wrapper with stdin at /dev/null and passing argv as an array; low risk, well-scoped, and the argv change also removes the prior shell-quoting/injection surface. No code changed since the last review pass (same commit), so the one prior non-blocking note on the environment guard still stands.

🐛 Fix — op item create/edit no longer hit the JSON-template stdin path; writes succeed again instead of leaking the rotated password to the terminal.
♻️ Refactor — 1Password helpers build an argv vector (_build_1password_command_args) instead of a shell string, dropping escapeshellarg/Process::fromShellCommandline.
🔧 Chore — PHP floor pinned to >=8.3, symfony re-pinned to the 7.4 line, install/self-update pass --ignore-platform-reqs, new pre-autoload environment-guard.php, WPCS formatting.
📝 Docs — README aligned to PHP 8.3.

environment-guard.php:L14 — The guard only enforces the project's own floor (PHP_VERSION_ID < 80300), but adding --ignore-platform-reqs to composer dump-autoload in install-osx/self-update.php suppresses composer's generated platform_check.php, which normally rejects a PHP older than the strictest installed dependency requires. So the scenario the docblock cites — "a package installed under --ignore-platform-reqs that needs a newer PHP" — would still pass this guard (8.3 >= 8.3) and fatal at runtime if any dependency ever required >=8.4. It is latent today because symfony is re-pinned to the 7.4 line, but consider softening the docblock claim or having the guard reflect the true dependency floor so the comment and behavior match.

🤖 Prompt for AI agents — fix all findings

Verify each finding against the current code and only fix it if
needed.

Suggestions:
In `environment-guard.php`:
- Line 14: The version check `if ( PHP_VERSION_ID < 80300 )`
  enforces only the project's own >=8.3 floor, while the
  docblock above claims the guard protects against "a package
  installed under --ignore-platform-reqs that needs a newer
  PHP". Because `install-osx` and `self-update.php` now run
  `composer dump-autoload -o --ignore-platform-reqs`, composer
  no longer emits `platform_check.php`, so this guard is the
  only runtime gate and it passes on PHP 8.3 even when an
  installed dependency requires >=8.4, causing a later fatal
  deep in that dependency. Either soften the docblock to state
  the guard checks only the project floor, or compute/hardcode
  the strictest installed PHP requirement in the version check
  so the comment and behavior agree; keep the extension-loaded
  foreach loop and the STDERR $problems messaging unchanged.
  This is latent while symfony stays on the 7.4 line, so treat
  it as non-blocking.

♻️ Previously flagged

Still outstanding

⚠️ environment-guard.php:L14 — guard enforces only the >=8.3 project floor while the docblock claims it catches dependencies needing newer PHP; code unchanged since the prior pass (same commit).

ℹ️ Review info

Commit: c1a72bf
Files reviewed: README.md, commands/GitHub_Checklist_Add.php, commands/WPCOM_Site_WP_User_Password_Rotate.php, composer.json, environment-guard.php, includes/functions-1password.php, includes/functions-github.php, install-osx, mcp-server.php, mcp/Team51McpTools.php, self-update.php, team51-cli.php
Files skipped: composer.lock (generated lockfile); the bulk of mcp/Team51McpTools.php, commands/GitHub_Checklist_Add.php, and includes/functions-github.php changes are phpcbf whitespace/alignment only (verified, no logic change)
Model: claude-opus-4-8

The docblock implied the guard caught a dependency installed under --ignore-platform-reqs that needs a newer PHP, but the check only enforces the project's own >=8.3 floor: on PHP 8.3 a dependency requiring 8.4 would still pass and fatal deep in that dependency. State that it checks the project floor only, so comment and behavior agree. Addresses PR #135 review. Assisted-by: Claude Code:claude-opus-4-8

ahegyes and others added 4 commits June 30, 2026 15:59

update packages

9915582

ahegyes changed the title ~~Fix/op 1password stdin quoting~~ Fix 1Password writes failing with "invalid JSON in piped input" Jun 30, 2026

ahegyes and others added 11 commits June 30, 2026 16:43

Fix spacing

044271e

Merge branch 'trunk' into fix/op-1password-stdin-quoting

c1a72bf

t51eng-poseidon Bot reviewed Jul 1, 2026

View reviewed changes

ahegyes merged commit 44f6664 into trunk Jul 2, 2026
4 of 6 checks passed

ahegyes deleted the fix/op-1password-stdin-quoting branch July 2, 2026 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix 1Password writes failing with "invalid JSON in piped input"#135

Fix 1Password writes failing with "invalid JSON in piped input"#135
ahegyes merged 16 commits into
trunkfrom
fix/op-1password-stdin-quoting

ahegyes commented Jun 30, 2026 •

edited

Loading

Uh oh!

t51eng-poseidon Bot left a comment

Uh oh!

t51eng-poseidon Bot Jul 1, 2026

Uh oh!

t51eng-poseidon Bot Jul 1, 2026

Uh oh!

t51eng-poseidon Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ahegyes commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Fix

Additional hardening (surfaced during review)

Testing

Uh oh!

t51eng-poseidon Bot left a comment

Choose a reason for hiding this comment

🔱 Poseidon Review

Uh oh!

t51eng-poseidon Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

t51eng-poseidon Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

t51eng-poseidon Bot left a comment

Choose a reason for hiding this comment

🔱 Poseidon Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ahegyes commented Jun 30, 2026 •

edited

Loading