Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions BUGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,31 @@ Dual ESM/CJS build via tsconfig.cjs.json + verify-cjs.cjs CI check.

## 🔄 IN PROGRESS

### PR #55 — Hybrid challenge missing access_token (2026-06-22 sprint, Choco)
**Branch:** `fix/hybrid-challenge-missing-token`
**PR:** https://github.com/dupe-com/botcha/pull/55

**Bug (2026-06-22): Hybrid challenge returns badge but no access_token**
The hybrid challenge is now the default (`GET /v1/challenges` returns hybrid by default).
On success it returned only a `badge` JWT — not a `botcha-verified` access_token + refresh_token.
The speed-only path correctly called `generateToken()`. All three hybrid handlers did not.
- **Root cause:** `verifyHybridChallenge` didn't propagate `app_id`; handlers never called `generateToken()`
- **Fix:** Propagate `app_id` in return value; add `generateToken()` to all 3 hybrid verify handlers
- **Tests:** `tests/unit/challenges/hybrid-token-issuance.test.ts` (3 tests, all passing)
### PR #56 — TAP session auth + totalTimeMs fix (2026-06-29 sprint, Choco)
**Branch:** `fix/tap-session-auth-and-timing`
**PR:** https://github.com/dupe-com/botcha/pull/56

**Bug A (2026-06-29): POST /v1/sessions/tap has no authentication — CRITICAL**
Any caller could create TAP sessions for arbitrary agents by supplying their agent_id in the body.
The tap_enabled gate was enforced but there was nothing stopping unauthenticated access.
- **Root cause:** `createTAPSessionRoute` never called `validateTAPAppAccess` unlike every other TAP route
- **Fix:** Add `validateTAPAppAccess(c, true)` at the top; verify app_id and agent_id match JWT claims
- **Tests:** 5 new auth enforcement tests (401, 403 cross-app, 403 agent_id mismatch, app-level token)

**Bug B (2026-06-29): totalTimeMs < speed.solveTimeMs in hybrid verify response**
`hybrid.issuedAt` was captured after two async KV writes, making totalTimeMs appear smaller than sub-challenge times — impossible math visible to every agent caller.
- **Root cause:** `issuedAt = Date.now()` placed after `await generateSpeedChallenge()` + `await generateReasoningChallenge()`
- **Fix:** Capture `issuedAt` before async operations; use it in both `issuedAt` and `expiresAt`
- **Tests:** 2 new timing invariant tests (totalTimeMs >= speed.solveTimeMs, >= reasoning.solveTimeMs)

---

## 🔮 TECHNICAL DEBT (existing in main, deprioritized)

### 7. POST /v1/sessions/tap requires agent_id in body (no JWT binding)
### 7. POST /v1/sessions/tap requires agent_id in body (no JWT binding) — FIXED in PR #56
**Location:** `tap-routes.ts` → `createTAPSessionRoute`
**Issue:** Handler requires `agent_id` in body but never validates it matches the authenticated agent's JWT claim. An agent with a valid token could theoretically pass a different agent_id — no access control check.
**Fix:** Extract `agent_id` from JWT payload (`c.get('tokenPayload')?.agent_id`); use it instead of / in addition to body `agent_id`. If both present, verify they match.
**Priority:** 🟠 MAJOR — authentication gap
**Fix:** `validateTAPAppAccess` added; app_id and agent_id cross-checked against JWT claims.
**PR:** https://github.com/dupe-com/botcha/pull/56

### 8. Capability format inconsistency across API surfaces
**Location:** `POST /v1/sessions/tap` vs `POST /v1/attestations` vs `POST /v1/delegations`
Expand Down
9 changes: 7 additions & 2 deletions packages/cloudflare-workers/src/challenges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,11 @@ export async function generateHybridChallenge(
cleanExpired();

const id = uuid();
// Capture issuedAt BEFORE any async operations so that totalTimeMs in the
// verify response correctly reflects the full elapsed time seen by the caller.
// Previously, issuedAt was stamped after two KV writes (~300-600ms), causing
// totalTimeMs to appear smaller than speed.solveTimeMs — impossible math.
const issuedAt = Date.now();

// Generate both sub-challenges (speed with RTT awareness)
const speedChallenge = await generateSpeedChallenge(kv, clientTimestamp, app_id);
Expand All @@ -1189,8 +1194,8 @@ export async function generateHybridChallenge(
id,
speedChallengeId: speedChallenge.id,
reasoningChallengeId: reasoningChallenge.id,
issuedAt: Date.now(),
expiresAt: Date.now() + 35000,
issuedAt,
expiresAt: issuedAt + 35000,
app_id,
};

Expand Down
32 changes: 32 additions & 0 deletions packages/cloudflare-workers/src/tap-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,18 @@ export async function listTAPAgentsRoute(c: Context) {
*/
export async function createTAPSessionRoute(c: Context) {
try {
// Authenticate the caller: requires a valid BOTCHA token (challenge-verified or agent-identity).
// Without this check, any unauthenticated caller could create TAP sessions for arbitrary agents
// by simply supplying their agent_id in the request body — a full authentication bypass.
const appAccess = await validateTAPAppAccess(c, true);
if (!appAccess.valid) {
return c.json({
success: false,
error: appAccess.error,
message: 'Authentication required. Include a valid BOTCHA bearer token.',
}, (appAccess.status || 401) as 401);
}

const body = await c.req.json().catch(() => ({}));

if (!body.agent_id || !body.user_context || !body.intent) {
Expand All @@ -465,6 +477,26 @@ export async function createTAPSessionRoute(c: Context) {

const agent = agentResult.agent!;

// Verify the agent belongs to the authenticated app.
// Prevents one app's token from creating sessions for another app's agents.
if (agent.app_id !== appAccess.appId) {
return c.json({
success: false,
error: 'UNAUTHORIZED',
message: 'Agent does not belong to this app',
}, 403);
}

// If the JWT carries a specific agent_id (agent-identity token), it MUST match
// the body agent_id. Prevents agent_A from creating sessions as agent_B.
if (appAccess.agentId && appAccess.agentId !== agent.agent_id) {
return c.json({
success: false,
error: 'AGENT_ID_MISMATCH',
message: 'The agent_id in the request body does not match the authenticated agent. An agent may only create sessions for itself.',
}, 403);
}

// Gate: agent must have TAP enabled (i.e. a registered public key).
// tap_enabled is set to true only when a public key is registered, so this
// check is the canonical guard. Without cryptographic identity, a TAP
Expand Down
176 changes: 176 additions & 0 deletions tests/unit/agents/tap-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,8 +618,184 @@ describe('TAP Routes - listTAPAgentsRoute', () => {
describe('TAP Routes - createTAPSessionRoute', () => {
beforeEach(() => {
vi.clearAllMocks();
// createTAPSessionRoute now requires a valid BOTCHA bearer token.
// Set up default auth mocks so existing tests continue to pass.
// Override per-test for auth-failure scenarios.
mockExtractBearerToken.mockReturnValue('mock-jwt-token');
mockVerifyToken.mockResolvedValue({
valid: true,
payload: {
type: 'botcha-agent-identity',
app_id: TEST_APP_ID,
agent_id: TEST_AGENT_ID,
},
});
});

// ---- Auth enforcement tests (added with fix/tap-session-auth-and-timing) ----

test('should return 401 when no bearer token provided', async () => {
mockExtractBearerToken.mockReturnValue(null);

const mockContext = createMockContext({
json: vi.fn().mockResolvedValue({
agent_id: TEST_AGENT_ID,
user_context: 'user_hash_123',
intent: { action: 'browse' },
}),
});

const response = await createTAPSessionRoute(mockContext);
const data = await response.json();

expect(response.status).toBe(401);
expect(data.success).toBe(false);
expect(data.error).toBe('UNAUTHORIZED');
});

test('should return 401 when bearer token is invalid', async () => {
mockExtractBearerToken.mockReturnValue('bad-token');
mockVerifyToken.mockResolvedValue({ valid: false });

const mockContext = createMockContext({
json: vi.fn().mockResolvedValue({
agent_id: TEST_AGENT_ID,
user_context: 'user_hash_123',
intent: { action: 'browse' },
}),
});

const response = await createTAPSessionRoute(mockContext);
const data = await response.json();

expect(response.status).toBe(401);
expect(data.success).toBe(false);
expect(data.error).toBe('INVALID_TOKEN');
});

test('should return 403 when agent does not belong to authenticated app', async () => {
// Token authenticates app_other, but agent belongs to TEST_APP_ID
mockVerifyToken.mockResolvedValue({
valid: true,
payload: {
type: 'botcha-agent-identity',
app_id: 'app_other_xyz',
agent_id: TEST_AGENT_ID,
},
});

const agentsKV = new MockKV();
const testAgent: TAPAgent = {
agent_id: TEST_AGENT_ID,
app_id: TEST_APP_ID, // belongs to TEST_APP_ID, not 'app_other_xyz'
name: 'TestAgent',
created_at: Date.now(),
tap_enabled: true,
capabilities: [{ action: 'browse', scope: ['*'] }],
};
agentsKV.seed(`agent:${TEST_AGENT_ID}`, testAgent);

const mockContext = createMockContext({
agentsKV,
json: vi.fn().mockResolvedValue({
agent_id: TEST_AGENT_ID,
user_context: 'user_hash_123',
intent: { action: 'browse' },
}),
});

const response = await createTAPSessionRoute(mockContext);
const data = await response.json();

expect(response.status).toBe(403);
expect(data.success).toBe(false);
expect(data.error).toBe('UNAUTHORIZED');
});

test('should return 403 when agent_id in body does not match JWT agent_id', async () => {
const OTHER_AGENT_ID = 'agent_other_9999';
// Token authenticates AGENT_A, but body requests session for AGENT_B
mockVerifyToken.mockResolvedValue({
valid: true,
payload: {
type: 'botcha-agent-identity',
app_id: TEST_APP_ID,
agent_id: OTHER_AGENT_ID, // JWT says other agent
},
});

const agentsKV = new MockKV();
const testAgent: TAPAgent = {
agent_id: TEST_AGENT_ID, // body says TEST_AGENT_ID
app_id: TEST_APP_ID,
name: 'TestAgent',
created_at: Date.now(),
tap_enabled: true,
capabilities: [{ action: 'browse', scope: ['*'] }],
};
agentsKV.seed(`agent:${TEST_AGENT_ID}`, testAgent);

const mockContext = createMockContext({
agentsKV,
json: vi.fn().mockResolvedValue({
agent_id: TEST_AGENT_ID,
user_context: 'user_hash_123',
intent: { action: 'browse' },
}),
});

const response = await createTAPSessionRoute(mockContext);
const data = await response.json();

expect(response.status).toBe(403);
expect(data.success).toBe(false);
expect(data.error).toBe('AGENT_ID_MISMATCH');
expect(data.message).toContain('may only create sessions for itself');
});

test('should allow app-level token (no agent_id in JWT) to create session for any agent in same app', async () => {
// App-level token (like a challenge-verified token) has no agent_id
mockVerifyToken.mockResolvedValue({
valid: true,
payload: {
type: 'botcha-verified',
app_id: TEST_APP_ID,
// no agent_id — app-level token
},
});

const agentsKV = new MockKV();
const sessionsKV = new MockKV();
const testAgent: TAPAgent = {
agent_id: TEST_AGENT_ID,
app_id: TEST_APP_ID,
name: 'TestAgent',
created_at: Date.now(),
tap_enabled: true,
capabilities: [{ action: 'browse', scope: ['products'] }],
};
agentsKV.seed(`agent:${TEST_AGENT_ID}`, testAgent);

const mockContext = createMockContext({
agentsKV,
sessionsKV,
json: vi.fn().mockResolvedValue({
agent_id: TEST_AGENT_ID,
user_context: 'user_hash_123',
intent: { action: 'browse' },
}),
});

const response = await createTAPSessionRoute(mockContext);
const data = await response.json();

// App-level tokens (no agent_id constraint) should still work
expect(response.status).toBe(201);
expect(data.success).toBe(true);
});

// ---- End auth enforcement tests ----

test('should create TAP session successfully', async () => {
const agentsKV = new MockKV();
const sessionsKV = new MockKV();
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/challenges/hybrid-token-issuance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,62 @@ describe('verifyHybridChallenge — app_id propagation', () => {
expect(result.app_id).toBe(APP_ID);
});
});

describe('verifyHybridChallenge — totalTimeMs correctness', () => {
/**
* Regression test for: hybrid.issuedAt was captured AFTER async sub-challenge
* generation, causing totalTimeMs < speed.solveTimeMs (physically impossible).
*
* Fix: capture issuedAt = Date.now() BEFORE generateSpeedChallenge() and
* generateReasoningChallenge() calls. Now totalTimeMs >= speed.solveTimeMs.
*/
it('totalTimeMs is >= speed.solveTimeMs (issuedAt captured before async ops)', async () => {
const kv = makeMockKV();
const challenge = await generateHybridChallenge(kv);

const speedAnswers = solveSpeed(challenge.speed.problems);

const stored = await kv.get(`hybrid:${challenge.id}`);
const hybridData: HybridChallenge = JSON.parse(stored);

const reasoningStored = await kv.get(`challenge:${hybridData.reasoningChallengeId}`);
const reasoningData = JSON.parse(reasoningStored);
const reasoningAnswers: Record<string, string> = {};
for (const [qid, accepted] of Object.entries(reasoningData.expectedAnswers as Record<string, string[]>)) {
reasoningAnswers[qid] = (accepted as string[])[0];
}

const result = await verifyHybridChallenge(challenge.id, speedAnswers, reasoningAnswers, kv);

expect(result.valid).toBe(true);
expect(result.totalTimeMs).toBeDefined();
expect(typeof result.totalTimeMs).toBe('number');

// The critical invariant: total wall-clock time must be >= the speed sub-test time.
// Before the fix, hybrid.issuedAt was stamped AFTER async sub-challenge creation,
// making totalTimeMs LESS than speed.solveTimeMs — physically impossible.
expect(result.totalTimeMs!).toBeGreaterThanOrEqual(result.speed.solveTimeMs ?? 0);
});

it('totalTimeMs is >= reasoning.solveTimeMs', async () => {
const kv = makeMockKV();
const challenge = await generateHybridChallenge(kv);

const speedAnswers = solveSpeed(challenge.speed.problems);

const stored = await kv.get(`hybrid:${challenge.id}`);
const hybridData: HybridChallenge = JSON.parse(stored);

const reasoningStored = await kv.get(`challenge:${hybridData.reasoningChallengeId}`);
const reasoningData = JSON.parse(reasoningStored);
const reasoningAnswers: Record<string, string> = {};
for (const [qid, accepted] of Object.entries(reasoningData.expectedAnswers as Record<string, string[]>)) {
reasoningAnswers[qid] = (accepted as string[])[0];
}

const result = await verifyHybridChallenge(challenge.id, speedAnswers, reasoningAnswers, kv);

expect(result.valid).toBe(true);
expect(result.totalTimeMs!).toBeGreaterThanOrEqual(result.reasoning.solveTimeMs ?? 0);
});
});
Loading