Source channel is read from {@link ChatOrigin} (carried on the - * {@link ToolContext}); when the context carries no origin it falls back to - * the legacy {@link ToolExecutionContext} username (treated as web/authenticated - * only if non-blank — preserves the original contract for the ThreadLocal path). + *
Trust is keyed off {@link ChatOrigin#channelType()}: only the explicit
+ * {@code "web"} channel (built by {@code ChatOrigin.web()}) may resolve to
+ * {@code authenticated} — every other channel is typed as {@code anonymous}
+ * / {@code external}, and an unknown (null/blank) channel resolves
+ * to {@link ResolvedIdentity#NONE} (fail-closed). The ThreadLocal
+ * {@link ToolExecutionContext} username is consulted only inside the web
+ * branch and only when no immutable user id is present.
*/
ResolvedIdentity classify(ToolContext ctx) {
ChatOrigin origin = ChatOrigin.from(ctx);
@@ -106,10 +121,19 @@ public class McpIdentityForwardService {
return ResolvedIdentity.NONE;
}
String channel = origin.channelType();
+ // Unknown / unattributed channel: never assert identity. An absent
+ // channelType must NOT be promoted to authenticated — only the explicit
+ // "web" channel (built by ChatOrigin.web()) carries MateClaw's assertion
+ // that a real account backs this request. Treating null/blank as web
+ // would silently stamp an untrusted ThreadLocal value with authenticated
+ // trust, violating the fail-closed contract this service guarantees.
+ if (channel == null || channel.isBlank()) {
+ return ResolvedIdentity.NONE;
+ }
// Authenticated web account: MateClaw vouches for this user. Prefer the
// immutable numeric id when available (web-console login); fall back to
// the username when only the ThreadLocal path supplied identity.
- if (channel == null || channel.isBlank() || "web".equals(channel)) {
+ if ("web".equals(channel)) {
if (origin.requesterUserId() != null) {
return new ResolvedIdentity(String.valueOf(origin.requesterUserId()),
TRUST_AUTHENTICATED, "web");
@@ -180,20 +204,35 @@ public class McpIdentityForwardService {
}
}
+ /**
+ * Resolve the signing key, parsing it lazily. Self-heals when the configured
+ * PEM changes (e.g. an operator fixes a malformed key or a config reload
+ * pushes a new one) — a prior failed parse is retried on the next call once
+ * {@code private-key-pem} differs from what was last attempted, so recovery
+ * no longer needs an app restart. Stays fail-closed otherwise.
+ */
private PrivateKey signingKey() {
PrivateKey k = signingKey;
if (k != null) {
return k;
}
- if (keyParseAttempted) {
- return null; // already tried and failed; don't spam parsing
+ String pem = properties.getToken().getPrivateKeyPem();
+ // Skip only while the PEM is unchanged since the last attempt — that
+ // avoids re-parsing (and re-logging) on every call. A changed PEM clears
+ // the way for a fresh parse, which is the self-healing path.
+ if (lastAttemptedPem != null && lastAttemptedPem.equals(pem)) {
+ return null;
}
synchronized (this) {
if (signingKey != null) {
return signingKey;
}
- keyParseAttempted = true;
- String pem = properties.getToken().getPrivateKeyPem();
+ // Re-check under the lock: another thread may have just attempted
+ // the same (unchanged) PEM.
+ if (lastAttemptedPem != null && lastAttemptedPem.equals(pem)) {
+ return null;
+ }
+ lastAttemptedPem = pem;
if (pem == null || pem.isBlank()) {
log.error("[McpIdentity] token mode enabled but mateclaw.mcp.identity-forward.token.private-key-pem is empty; "
+ "identity tokens will NOT be issued (fail-closed)");
diff --git a/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java b/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java
index e012a7df..8aea890f 100644
--- a/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/tool/mcp/runtime/McpIdentityForwardServiceTest.java
@@ -129,6 +129,51 @@ class McpIdentityForwardServiceTest {
assertThat(id.subject()).isEqualTo("alice");
assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_AUTHENTICATED);
}
+
+ // ---- Fail-closed: an unknown / unattributed channel must NEVER be
+ // promoted to authenticated, even when a stale ThreadLocal username
+ // is present. This is the regression for the privilege-escalation
+ // gap where channel==null used to fall into the web branch and
+ // stamp an untrusted identifier with authenticated trust. ----
+
+ @Test
+ @DisplayName("unknown channelType (null) → NONE, even with a polluted ThreadLocal (no privilege escalation)")
+ void unknownChannelIsFailClosedEvenWithThreadLocal() {
+ // A stale username from a prior request reused this thread.
+ ToolExecutionContext.set("c1", "attacker");
+ // System-style origin built with no channelType (e.g. SkillConsolidation
+ // / SkillReflection internal tasks): requesterId="", channelType=null.
+ ChatOrigin origin = new ChatOrigin(null, "c1", "", 1L, null,
+ null, null, false, null, null, null, null, null);
+ assertThat(svc(new McpIdentityForwardProperties()).classify(ctx(origin)))
+ .isEqualTo(McpIdentityForwardService.ResolvedIdentity.NONE);
+ }
+
+ @Test
+ @DisplayName("blank channelType → NONE (fail-closed, not authenticated)")
+ void blankChannelIsFailClosed() {
+ ToolExecutionContext.set("c1", "attacker");
+ ChatOrigin origin = new ChatOrigin(null, "c1", "someone", 1L, null,
+ null, null, false, null, " ", null, null, null);
+ assertThat(svc(new McpIdentityForwardProperties()).classify(ctx(origin)))
+ .isEqualTo(McpIdentityForwardService.ResolvedIdentity.NONE);
+ }
+
+ @Test
+ @DisplayName("unrecognised non-web channel → downgraded to external (never authenticated)")
+ void novelChannelIsDowngradedNotAuthenticated() {
+ // A channel the classifier doesn't recognise is treated as external
+ // (explicit trust downgrade), never as authenticated. The backend sees
+ // an untrusted id and decides for itself — this is the key guarantee:
+ // no unknown channel can ever acquire authenticated trust.
+ ChatOrigin origin = new ChatOrigin(null, "c1", "u1", 1L, null,
+ null, null, false, null, "future-net", null, null, null);
+ McpIdentityForwardService.ResolvedIdentity id =
+ svc(new McpIdentityForwardProperties()).classify(ctx(origin));
+ assertThat(id.subject()).isEqualTo("u1");
+ assertThat(id.trust()).isEqualTo(McpIdentityForwardService.TRUST_EXTERNAL);
+ assertThat(id.channelType()).isEqualTo("future-net");
+ }
}
// ==================== Resolution: plaintext & token modes ====================
@@ -170,6 +215,41 @@ class McpIdentityForwardServiceTest {
assertThat(svc(p).resolve(ctx(origin), "my-api")).isEmpty();
}
+ @Test
+ @DisplayName("signing key self-heals after a bad PEM is corrected (no restart needed)")
+ void signingKeySelfHealsAfterConfigFix() {
+ KeyPair kp = rsaKeyPair();
+ String goodPem = Base64.getEncoder().encodeToString(kp.getPrivate().getEncoded());
+ ChatOrigin origin = ChatOrigin.web("c1", "alice", 1L, null, null, 42L);
+
+ var p = new McpIdentityForwardProperties();
+ p.getToken().setEnabled(true);
+ p.getToken().setIssuer("mateclaw");
+ p.getToken().setTtlSeconds(60);
+
+ // 1. Malformed key → fail-closed.
+ p.getToken().setPrivateKeyPem("not-a-valid-pem");
+ McpIdentityForwardService service = svc(p);
+ assertThat(service.resolve(ctx(origin), "my-api")).isEmpty();
+
+ // 2. Operator fixes the config (or a reload pushes a good key) → next
+ // call re-parses and issues a token, without needing an app restart.
+ p.getToken().setPrivateKeyPem(goodPem);
+ Optional