fix(security): harden item_get, vault allow-list, op_run, and CI (v4.0.4)

.github/workflows/ci.yml

@@ -20,6 +20,8 @@ jobs:
 
     steps:
       - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
 
       - name: Use Node.js ${{ matrix.node-version }}
         uses: actions/setup-node@v4

.github/workflows/publish.yml

@@ -7,34 +7,36 @@ on:
 
 permissions:
   contents: read
-  id-token: write
 
 jobs:
-  publish:
+  # Runs dependency code (npm ci, build, tests) with a read-only token and no
+  # OIDC access, then hands the packed tarball to the publish job.
+  build:
+    name: Build, test, and pack
     runs-on: ubuntu-latest
     if: github.event_name == 'release' || github.event_name == 'workflow_dispatch'
+    permissions:
+      contents: read
+    outputs:
+      package_name: ${{ steps.pack.outputs.package_name }}
+      package_version: ${{ steps.pack.outputs.package_version }}
 
     steps:
-      - uses: actions/checkout@v6
+      - name: Check out repository
+        uses: actions/checkout@v6
+        with:
+          persist-credentials: false
 
       - name: Use Node.js 24
         uses: actions/setup-node@v6
         with:
           node-version: 24
-          registry-url: https://registry.npmjs.org
           package-manager-cache: false
 
-      - name: Verify trusted-publishing toolchain
-        run: |
-          node --version
-          npm --version
-          node -e "const [major, minor] = process.versions.node.split('.').map(Number); if (major < 22 || (major === 22 && minor < 14)) process.exit(1)"
-          npm install --global npm@latest
-          npm --version
-          node -e "const { execSync } = require('child_process'); const version = execSync('npm --version', { encoding: 'utf8' }).trim().split('.').map(Number); if (version[0] < 11 || (version[0] === 11 && version[1] < 5) || (version[0] === 11 && version[1] === 5 && version[2] < 1)) process.exit(1)"
-
+      # Dependency install scripts never run in the job that builds the
+      # published tarball; build and tests do not need them.
       - name: Install dependencies
-        run: npm ci
+        run: npm ci --ignore-scripts
 
       - name: Validate version alignment
         run: |
@@ -53,9 +55,10 @@ jobs:
 
       - name: Validate release tag matches package version
         if: github.event_name == 'release'
+        env:
+          RELEASE_TAG: ${{ github.event.release.tag_name }}
         run: |
           PACKAGE_VERSION=$(node -p "require('./package.json').version")
-          RELEASE_TAG="${{ github.event.release.tag_name }}"
           EXPECTED_TAG="v${PACKAGE_VERSION}"
 
           echo "release tag: ${RELEASE_TAG}"
@@ -66,12 +69,81 @@ jobs:
             exit 1
           fi
 
-      - name: Check if version already exists on npm
-        id: npm_check
+      - name: Build
+        run: npm run build
+
+      - name: Test
+        run: npm test
+
+      - name: Pack npm tarball
+        id: pack
         run: |
           PACKAGE_NAME=$(node -p "require('./package.json').name")
           PACKAGE_VERSION=$(node -p "require('./package.json').version")
 
+          # Pack into a fresh directory so the new tarball is the only .tgz
+          # there, whatever npm names it, then give it a fixed name.
+          PACK_DIR=$(mktemp -d)
+          npm pack --ignore-scripts --pack-destination "${PACK_DIR}"
+
+          set -- "${PACK_DIR}"/*.tgz
+          if [ "$#" -ne 1 ] || [ ! -f "$1" ]; then
+            echo "Expected exactly one tarball from npm pack."
+            exit 1
+          fi
+          mv "$1" package.tgz
+          echo "Packed ${PACKAGE_NAME}@${PACKAGE_VERSION} as package.tgz"
+
+          echo "package_name=${PACKAGE_NAME}" >> "$GITHUB_OUTPUT"
+          echo "package_version=${PACKAGE_VERSION}" >> "$GITHUB_OUTPUT"
+
+      - name: Upload npm package artifact
+        uses: actions/upload-artifact@v7
+        with:
+          name: npm-package
+          path: package.tgz
+          if-no-files-found: error
+          retention-days: 1
+
+  # The only job with OIDC access (npm trusted publishing). It never checks out
+  # the repository or installs/runs dependency code; it only publishes the
+  # tarball built above.
+  publish:
+    name: Publish to npm
+    needs: build
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      id-token: write
+
+    steps:
+      - name: Use Node.js 24
+        uses: actions/setup-node@v6
+        with:
+          node-version: 24
+          registry-url: https://registry.npmjs.org
+          package-manager-cache: false
+
+      - name: Verify trusted-publishing toolchain
+        run: |
+          node --version
+          npm --version
+          node -e "const [major, minor] = process.versions.node.split('.').map(Number); if (major < 22 || (major === 22 && minor < 14)) process.exit(1)"
+          npm install --global npm@latest
+          npm --version
+          node -e "const { execSync } = require('child_process'); const version = execSync('npm --version', { encoding: 'utf8' }).trim().split('.').map(Number); if (version[0] < 11 || (version[0] === 11 && version[1] < 5) || (version[0] === 11 && version[1] === 5 && version[2] < 1)) process.exit(1)"
+
+      - name: Download npm package artifact
+        uses: actions/download-artifact@v8
+        with:
+          name: npm-package
+
+      - name: Check if version already exists on npm
+        id: npm_check
+        env:
+          PACKAGE_NAME: ${{ needs.build.outputs.package_name }}
+          PACKAGE_VERSION: ${{ needs.build.outputs.package_version }}
+        run: |
           if npm view "${PACKAGE_NAME}@${PACKAGE_VERSION}" version >/dev/null 2>&1; then
             echo "already_published=true" >> "$GITHUB_OUTPUT"
             echo "Version ${PACKAGE_VERSION} is already published. Skipping publish."
@@ -80,12 +152,6 @@ jobs:
             echo "Version ${PACKAGE_VERSION} is not published yet."
           fi
 
-      - name: Build
-        run: npm run build
-
-      - name: Test
-        run: npm test
-
       - name: Publish to npm with OIDC
         if: steps.npm_check.outputs.already_published == 'false'
-        run: npm publish --access public
+        run: npm publish ./package.tgz --access public --ignore-scripts

CHANGELOG.md

@@ -5,6 +5,30 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [4.0.4] - 2026-10-04
+
+### Security
+
+- **`item_get` no longer returns secrets held in non-`Concealed` fields** — Only `Concealed` fields were masked, so SSH private keys, one-time-password (TOTP) seeds, and card numbers came back in plaintext without `reveal: true`. Masking is now deny-by-default: only known non-secret field types (text, URL, email, phone, date, month/year, menu, card type, address, reference) are shown, and every other type, including any added by future SDK versions, needs `reveal: true`. Notes are still returned as-is, matching `op item get`; don't keep secrets in the notes of vaults an agent can read.
+- **Vault allow-list is now enforced server-wide** — `OP_MCP_ALLOWED_VAULTS` / `--allowed-vaults` only covered `op_run` and `op_check_ref`, and only compared the vault segment as written in an `op://` reference. It now applies to every tool and resource that touches a vault: `vault_list` and `1password://vaults` are filtered, and vault IDs are checked before anything is returned or modified. `op://` references are checked both as written and by the vault they actually resolve to. It fails closed if vaults can't be listed.
+- **`op_run` redaction no longer leaks the remainder of overlapping secrets** — Redaction now masks every occurrence of every secret in a single pass over the original output, so overlapping or nested secrets can't leak: a username that is a prefix of a credential string is no longer replaced first, leaving the password visible. It also masks common encodings of each secret: base64 (including inside a larger base64 payload such as an HTTP Basic auth header), JSON-escaped and URL-encoded forms, and multi-line secrets line by line and with CRLF line endings.
+- **`op_run` output cap no longer exhausts memory** — The 5 MiB per-stream cap is now applied while the command runs. Excess output is discarded instead of buffered, which fixes a memory-exhaustion crash, and a secret cut off at the cap never survives as a partial prefix.
+- **`op_run` timeouts kill the whole process tree** — On timeout the process group (POSIX) or process tree via `taskkill /T` (Windows) is killed, and `op_run` always returns shortly afterwards, even if a background process holds the output pipes. Previously it could hang forever and leave processes running with injected secrets.
+- **Case-insensitive credential scrub in `op_run`** — The server's own credential variables (`OP_SERVICE_ACCOUNT_TOKEN`, `OP_KEYCHAIN_SERVICE`, `OP_KEYCHAIN_ACCOUNT`) are now stripped from the child environment regardless of letter case.
+- **Removed a leftover CI workflow and hardened publishing** — The one-time `mcp-v2-migration.yml` is deleted: any GitHub user could trigger it with a PR comment, and it ran `npm install` with a write-scoped token. `publish.yml` no longer interpolates the release tag into shell, checkouts no longer persist credentials, and publishing is split into two jobs: build, test, and pack run without OIDC access (and without dependency install scripts), and a separate job that alone has `id-token: write` publishes the prebuilt tarball with `--ignore-scripts`.
+- **macOS Keychain lookup uses an absolute path** — The token lookup runs `/usr/bin/security` instead of resolving `security` through `PATH`, so a binary planted earlier on `PATH` can't hand the server an attacker-chosen service account token.
+- **Warning when the token is passed on the command line** — `--service-account-token` / `--token` put the token in the process arguments, which other local processes (including commands run through `op_run`) can read. The server now logs a startup warning; prefer `OP_SERVICE_ACCOUNT_TOKEN` or, on macOS, the Keychain.
+- All of the above came out of an internal security review.
+
+### Changed
+
+- **Behavior change: the vault allow-list is server-wide** — If you set `OP_MCP_ALLOWED_VAULTS` / `--allowed-vaults` expecting it to affect only `op_run` and `op_check_ref`, it now restricts every tool and resource. Tools that take a `vaultId` need the vault's ID, not its name. With an allow-list set, each guarded call makes one extra `vaults.list` read (mind service account rate limits). It is defense in depth; scope the service account's own vault access in 1Password first.
+- **`item_get` hides more by default** — SSH private keys, OTP seeds, and card numbers now show `[concealed]` unless you pass `reveal: true`.
+- **Stricter `op://` validation** — Malformed references passed to `item_get` and `password_read` are now rejected locally with a clear error.
+- **More candid `op_run` description** — The tool description no longer claims plaintext is "NEVER" returned. Redaction is best effort and protects against accidental disclosure; it is not a sandbox, because a command can deliberately transform or transmit a secret it was given.
+- **Publishing** — `prepublishOnly` now only matters for manual `npm publish`; the automated workflow publishes a prebuilt tarball.
+- **Documentation** — README, CONTRIBUTING, and agents.md cover the server-wide allow-list, best-effort redaction, unconcealed notes, and the two-job publish workflow.
+
 ## [4.0.3] - 2026-10-04
 
 ### Security