[navFooh]

Updates packages/mppx-stake from mppx-stake@0.7.0.

[gitbond-gatekeeper]

Bond No Longer Required

This PR was closed without a bond being activated. No bond is required.

[navFooh]

@claude can you review this PR for bugs and security issues?

[claude]

Claude finished @navFooh's task in 3m 8s —— View job

---

PR Review: Update mppx-stake to 0.7.0

• Gather context
• Review security-sensitive files (server/stake.ts, challenge.ts, config.ts)
• Review client-side changes
• Review CLI changes
• Post comprehensive review

---

Overall this PR is a clean simplification: hardcoding evm/stake as the method name, removing the user-configurable methodName parameter, and improving the consumeChallenge callback contract. No critical security vulnerabilities were introduced. Below are the findings, ordered by severity.

---

Moderate

1. Host Header Injection in challenge URL construction (pre-existing, not worsened)

File: apps/mpp-server/src/web.ts:28-34

const host =
  typeof hostHeader === 'string' && hostHeader.length > 0
    ? hostHeader
    : `${server.host}:${server.port}`

return `${req.protocol}://${host}`

getOrigin trusts the incoming Host header over the configured server binding. An attacker who can set Host: attacker.example.com in their request will see that origin embedded in the challenge (as the realm). Depending on how the mppx framework derives the challenge's realm, this could allow a client to obtain a challenge whose realm points to an attacker-controlled domain. This is not introduced by this PR (the diff only reformats the function signature), but it warrants a fix: validate that the Host header matches the configured config.host before using it.

Fix this →

---

Low

2. consumeChallenge defaults to no-op — replay window remains open

File: packages/mppx-stake/src/server/stake.ts:61-63

The PR improves the callback by passing { id, expires } so implementers can size their TTL precisely — that's a good change. However, the default is still a no-op:

consumeChallenge?:
  | ((context: { id: string; expires: string }) => Promise<void>)
  | undefined

Without replay protection, a valid credential can be submitted multiple times within the expires window. The documentation clearly warns about this, but because the out-of-box behaviour is insecure, it's easy for a demo deployment to accidentally go to production without this wired up. Consider logging a warning at server startup when consumeChallenge is not configured.

3. Breaking change to method name with no migration path

Files: packages/mppx-stake/src/method.ts:102-104, config.json

createStakeMethod() now hardcodes name: 'evm'. Any saved challenges or credentials issued under the previous method name (e.g. tempo/stake) will be rejected by parseStakeChallenge:

if (parsed.method !== method.name || parsed.intent !== method.intent)
  throw new Error(
    `Expected a ${method.name}/${method.intent} challenge, received ${parsed.method}/${parsed.intent}.`,
  )

For rolling deployments (server updated before all clients), there is a window where old clients holding tempo/stake challenges cannot complete the credential flow. No migration path or compatibility shim is documented. This is a known breaking change in 0.7.0, but the release notes / README should call it out explicitly with instructions for operators to invalidate any in-flight challenges.

4. Unsound as CredentialChallenge cast in CLI respond command

File: apps/cli/src/commands/challenge.ts:119-123

type CredentialChallenge = Parameters<
  typeof method.createCredential
>[0]['challenge']
const serializedCredential = await method.createCredential({
  challenge: challengeValue as CredentialChallenge,
})

The same workaround exists in client/stake.test.ts:45-52 with a comment explaining the cause (Challenge.fromMethod widens intent/method to string, erasing the literal types that createCredential's parameter depends on). In the test file this is acceptable; in the production command it is also safe because resolveStakeChallengeForRespond always returns a StakeChallenge or throws. The cast should carry the same explanatory comment as the test file to prevent future misuse.

Fix this →

---

Positive security changes in this PR

• consumeChallenge receives expires — implementers can now use a precise, challenge-scoped TTL instead of guessing.
• Guard for missing expires before consuming (stake.ts:136-139) — prevents a degenerate case where the store is called without a TTL anchor.
• HMAC verification runs before consumeChallenge — slot consumption only happens for credentials that actually originated from this server.
• ecdsaSignature() validator in method.ts:15-23 — rejects signatures that are only 0x-prefixed hex (the previous z.signature() would accept short or malformed values).
• assertRequestMatches checks all relevant fields (contract, counterparty, token, amount, resource, scope, chainId) — solid defence against parameter-confusion attacks.

---