Skip to content

fix(plugins): prevent secret-ref diagnostic reflection (BLO-20871) - #1221

Merged
kkroo merged 1 commit into
masterfrom
codex/supersede-pr-968
Aug 11, 2026
Merged

fix(plugins): prevent secret-ref diagnostic reflection (BLO-20871)#1221
kkroo merged 1 commit into
masterfrom
codex/supersede-pr-968

Conversation

@kkroo

@kkroo kkroo commented Aug 9, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open-source control plane for managing AI agents at work.
  • Plugin configuration is company-scoped operational state and can contain inline credentials or secret references.
  • The reviewed fix(plugins): mask plugin-config secrets on read, lossless masked round-trip (BLO-20871) #968 change closes the read-authority and config-response disclosure boundary while preserving masked round trips.
  • Its final review found one remaining path: a valid secret-reference object could carry an extra plaintext field through config test validation and worker diagnostics.
  • This independent successor reapplies the reviewed boundary work from current master and makes pointer payloads fail closed before they cross that boundary.
  • The result keeps direct configuration values usable by the worker while preventing discarded pointer payload fields from reaching it or appearing in diagnostics.

Linked Issues or Issue Description

Fixes BLO-20871 and BLO-20794. Supersedes app-authored #968 without modifying it.

What Changed

  • Reapplied the reviewed plugin-config authorization, masking, lossless round-trip, schema traversal, and diagnostic-redaction change set as a single kkroo-authored commit.
  • Canonicalized valid secret_ref and user_secret_ref objects before validation, persistence, and worker RPC.
  • Added discarded pointer payload leaves to config-test diagnostic redaction alongside direct secret values.
  • Added unit and route regressions for pointer payloads, direct values, warnings, and thrown worker errors.

Verification

  • pnpm exec vitest run server/src/tests/plugin-config-masking.test.ts server/src/tests/plugin-config-validator.test.ts server/src/tests/plugin-routes-authz.test.ts
  • 151 focused tests passed.
  • pnpm --filter @paperclipai/shared typecheck
  • pnpm --filter @paperclipai/server typecheck
  • Both typechecks passed.
  • The new regressions failed against the reconstructed fix(plugins): mask plugin-config secrets on read, lossless masked round-trip (BLO-20871) #968 head before the fix: the pointer value reached the worker and was absent from the redaction set.

Risks

  • Low and deliberately narrowing. Valid pointer payloads now retain only schema-owned fields before worker validation or persistence.
  • Direct inline secret values remain available to workers as before, while warning, error, and RPC-error output is redacted.
  • No plugin manifest, credential store, migration, or public disclosure surface is broadened.

Model Used

OpenAI Codex using GPT-5 with repository inspection, GitHub review analysis, isolated execution, and test tooling.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either linked existing issues or described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

Independently preserve the reviewed plugin configuration masking, authority, round-trip, and diagnostic-redaction changes from #968. Canonicalize valid secret-reference pointer input before validation, persistence, and worker RPC, and retain discarded pointer leaves in config-test diagnostic redaction alongside direct secret values.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-20794
🔗 Paperclip issue: BLO-20871

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-20794
🔗 Paperclip issue: BLO-20871

@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@kkroo

kkroo commented Aug 9, 2026

Copy link
Copy Markdown
Author

@ally please review exact head ac2e3d2.

This is an independent kkroo-authored successor to app-authored #968, rooted at current master and preserving its reviewed eight-file authorization, masking, round-trip, and diagnostic-redaction change set.

The final unresolved Important finding is addressed with two defenses:

  1. Valid secret_ref and user_secret_ref input is canonicalized to schema-owned fields before validation, persistence, or worker RPC, so an extra plaintext field cannot reach the worker.
  2. Discarded pointer payload string leaves are retained in the config-test redaction set together with direct secret values, covering warnings and thrown RPC errors defensively.

Focused evidence at this exact head:

Please provide the formal App exact-head review. The allyblockcast User-seat review has been requested separately.

@kkroo
kkroo requested a review from allyblockcast August 9, 2026 14:08

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: ac2e3d2

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • Masks and canonicalizes secret-bearing configuration at the API boundary while preserving safe pointer metadata and lossless unchanged updates.
  • Redacts submitted and restored plaintext from worker warnings, validation errors, and structured bridge-error payloads.
  • Tests cover nested schema shapes, ref resolution, array identity matching, authorization, and JSON-metacharacter credentials.

Recommended Action

  1. No blocking issues found in this review.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: ac2e3d2

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • Masks and canonicalizes secret-bearing configuration at the API boundary while preserving safe pointer metadata and lossless unchanged updates.
  • Redacts submitted and restored plaintext from worker warnings, validation errors, and structured bridge-error payloads.
  • Tests cover nested schema shapes, ref resolution, array identity matching, authorization, and JSON-metacharacter credentials.

Recommended Action

  1. No blocking issues found in this review.

@kkroo
kkroo added this pull request to the merge queue Aug 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 11, 2026
@kkroo
kkroo added this pull request to the merge queue Aug 11, 2026
Merged via the queue into master with commit 4e4bc4d Aug 11, 2026
44 of 48 checks passed
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.

2 participants