Skip to content

Protect secret placeholders in workflow artifacts#38861

Merged
terrymanu merged 2 commits into
apache:masterfrom
terrymanu:dev
Jun 17, 2026
Merged

Protect secret placeholders in workflow artifacts#38861
terrymanu merged 2 commits into
apache:masterfrom
terrymanu:dev

Conversation

@terrymanu

@terrymanu terrymanu commented Jun 17, 2026

Copy link
Copy Markdown
Member
  • Treat secret_ref as a protected placeholder input slot instead of a resolver-backed reference
  • Emit only masked values or neutral placeholders in plan, preview, validation, and manual artifacts
  • Stop automatic apply before side effects when placeholders still require manual replacement

For #35294

- Treat secret_ref as a protected placeholder input slot instead of a resolver-backed reference
- Emit only masked values or neutral placeholders in plan, preview, validation, and manual artifacts
- Stop automatic apply before side effects when placeholders still require manual replacement
- Treat secret_ref as a protected placeholder input slot instead of a resolver-backed reference
- Emit only masked values or neutral placeholders in plan, preview, validation, and manual artifacts
- Stop automatic apply before side effects when placeholders still require manual replacement
@terrymanu

Copy link
Copy Markdown
Member Author

Summary

Evidence

  • The PR body now uses a non-closing reference, For #35294; Collect ideas with ShardingSphere and MCP #35294 remains an open umbrella issue for MCP ideas, so this PR no longer claims to fully close that broad scope.
  • mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/model/SecretReferenceValue.java:31 keeps only placeholder state and malformed, not raw secret_ref values or user labels.
  • mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowSecretReferenceUtils.java:50 converts protected placeholder objects into internal placeholders, and mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowSecretReferenceUtils.java:175 rejects unresolved placeholders, neutral manual placeholders, blank values, and missing values during validation.
  • mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowArtifactMaskUtils.java:65 replaces internal placeholders with manual placeholders for review artifacts, while property maps still mask placeholder-backed sensitive properties.
  • mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionService.java:287 blocks automatic execution before side effects whenever secret placeholders remain.
  • Regression coverage is present at unit and E2E levels:
    • mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowSecretReferenceUtilsTest.java:50 verifies raw secret_ref and labels are not echoed.
    • mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowSecretReferenceUtilsTest.java:103 verifies unresolved and neutral placeholders are rejected.
    • mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionServiceTest.java:383 verifies automatic apply returns recovery and never executes artifacts.
    • test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/runtime/production/HttpProductionProxySecretReferenceWorkflowE2ETest.java:52 verifies the production Proxy workflow path has no applied artifacts when placeholders remain.
  • User docs are aligned with the code boundary: secret_ref is documented as a placeholder slot, not a resolver, and troubleshooting no longer lists this as an unresolved code problem.

Review Details

  • Reviewed Scope: Latest PR head cb52d622cef558285a45690e2106925d57ef94ff, merge-base 1cdbf585ca7616f7c6a1e0c9cd57f4ff1d73ffa4; GitHub /pulls/38861/files matched the local three-dot diff, 54 files. Reviewed MCP workflow code, encrypt/mask planning and validation paths, MCP descriptors, user docs, unit tests, and MCP E2E additions.
  • Not Reviewed Scope: GitHub Actions status/check-runs, non-MCP modules outside the PR diff, and the full broader Collect ideas with ShardingSphere and MCP #35294 MCP idea backlog beyond this PR's non-closing reference.
  • Verification: Rechecked latest PR metadata and file boundary locally; local worktree was clean and local HEAD matched the PR head. Maven was not rerun in this pass because the PR head SHA did not change and the update was PR text only; previous same-head local verification covered the MCP E2E and repository style gates.
  • Release Note / User Docs: User docs are required and verified in docs/document/content/user-manual/shardingsphere-mcp/. I do not require RELEASE-NOTES.md for this scoped MCP workflow safety/docs correction because it does not add a new released configuration key, dependency, packaging change, SQL syntax, or migration step beyond the updated MCP user documentation.

@terrymanu terrymanu merged commit 96b8227 into apache:master Jun 17, 2026
18 checks passed
@terrymanu terrymanu deleted the dev branch June 17, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants