openclaw/clawhub
1
0
Fork 0
mirror of https://github.com/openclaw/clawhub.git synced 2026-07-21 19:25:25 -04:00
Code Issues 183 Packages Projects Releases 1 Wiki Activity

CLI delete returns 'Unauthorized' for all errors (including 'Skill not found') #15

New Issue
Closed
opened 2026-02-15 17:15:04 -05:00 by yindo · 1 comment
yindo commented 2026-02-15 17:15:04 -05:00
Owner
Copy Link
Copy Source

Originally created by @sergical on GitHub (Jan 25, 2026).

Problem

The clawdhub delete <slug> command returns Unauthorized even when logged in as the skill owner.

Reproduction

clawdhub whoami
# ✔ sergical

clawdhub delete reflect-notes --yes
# ✖ Unauthorized

Root Cause

In convex/httpApiV1.ts line 471-489, the delete handler catches all errors and returns 401:

async function skillsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) {
  // ...
  try {
    const { userId } = await requireApiTokenUser(ctx, request)
    await ctx.runMutation(internal.skills.setSkillSoftDeletedInternal, {
      userId,
      slug,
      deleted: true,
    })
    return json({ ok: true }, 200, rate.headers)
  } catch {
    return text('Unauthorized', 401, rate.headers)  // <-- catches everything
  }
}

The mutation setSkillSoftDeletedInternal can throw several errors:

  • User not found
  • Slug required
  • Skill not found
  • Role assertion failure (for non-owners)

All of these currently surface as Unauthorized, which is misleading.

Suggested Fix

Differentiate between auth errors and other errors:

async function skillsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) {
  const rate = await applyRateLimit(ctx, request, 'write')
  if (!rate.ok) return rate.response

  const segments = getPathSegments(request, '/api/v1/skills/')
  if (segments.length !== 1) return text('Not found', 404, rate.headers)
  const slug = segments[0]?.trim().toLowerCase() ?? ''
  
  let userId: Id<'users'>
  try {
    const auth = await requireApiTokenUser(ctx, request)
    userId = auth.userId
  } catch {
    return text('Unauthorized', 401, rate.headers)
  }
  
  try {
    await ctx.runMutation(internal.skills.setSkillSoftDeletedInternal, {
      userId,
      slug,
      deleted: true,
    })
    return json({ ok: true }, 200, rate.headers)
  } catch (err) {
    const message = err instanceof Error ? err.message : 'Unknown error'
    if (message.includes('not found')) {
      return text('Not found', 404, rate.headers)
    }
    if (message.includes('Only the owner') || message.includes('assertRole')) {
      return text('Forbidden', 403, rate.headers)
    }
    return text(message, 400, rate.headers)
  }
}

This pattern should probably be applied to other handlers that have the same issue (undelete, publish, etc.).

Originally created by @sergical on GitHub (Jan 25, 2026). ## Problem The `clawdhub delete <slug>` command returns `Unauthorized` even when logged in as the skill owner. ## Reproduction ```bash clawdhub whoami # ✔ sergical clawdhub delete reflect-notes --yes # ✖ Unauthorized ``` ## Root Cause In `convex/httpApiV1.ts` line 471-489, the delete handler catches **all** errors and returns 401: ```ts async function skillsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) { // ... try { const { userId } = await requireApiTokenUser(ctx, request) await ctx.runMutation(internal.skills.setSkillSoftDeletedInternal, { userId, slug, deleted: true, }) return json({ ok: true }, 200, rate.headers) } catch { return text('Unauthorized', 401, rate.headers) // <-- catches everything } } ``` The mutation `setSkillSoftDeletedInternal` can throw several errors: - `User not found` - `Slug required` - `Skill not found` - Role assertion failure (for non-owners) All of these currently surface as `Unauthorized`, which is misleading. ## Suggested Fix Differentiate between auth errors and other errors: ```ts async function skillsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) { const rate = await applyRateLimit(ctx, request, 'write') if (!rate.ok) return rate.response const segments = getPathSegments(request, '/api/v1/skills/') if (segments.length !== 1) return text('Not found', 404, rate.headers) const slug = segments[0]?.trim().toLowerCase() ?? '' let userId: Id<'users'> try { const auth = await requireApiTokenUser(ctx, request) userId = auth.userId } catch { return text('Unauthorized', 401, rate.headers) } try { await ctx.runMutation(internal.skills.setSkillSoftDeletedInternal, { userId, slug, deleted: true, }) return json({ ok: true }, 200, rate.headers) } catch (err) { const message = err instanceof Error ? err.message : 'Unknown error' if (message.includes('not found')) { return text('Not found', 404, rate.headers) } if (message.includes('Only the owner') || message.includes('assertRole')) { return text('Forbidden', 403, rate.headers) } return text(message, 400, rate.headers) } } ``` This pattern should probably be applied to other handlers that have the same issue (undelete, publish, etc.).
yindo closed this issue 2026-02-15 17:15:05 -05:00
yindo commented 2026-02-15 17:15:05 -05:00
Author
Owner
Copy Link
Copy Source

@sergical commented on GitHub (Jan 25, 2026):

Update: Additional Root Cause Found

After further investigation, we found that the "Unauthorized" error was actually masking a CLI timeout issue.

What Actually Happened

  1. The delete API was working correctly (verified via direct curl call)
  2. The CLI has a 15-second timeout (REQUEST_TIMEOUT_MS = 15_000)
  3. When the server is slow (cold start?), the timeout fires
  4. The CLI throws controller.abort('Timeout') — but passing a string instead of an Error
  5. pRetry wraps this in: Non-error was thrown: "Timeout". You should only throw errors.
  6. This gets caught and displayed, but the confusing error message makes it hard to diagnose

CLI Bug Location

// packages/clawdhub/src/http.ts (or dist/http.js)
const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS);

When controller.abort() is called with a string, the AbortSignal throws the string itself (not an Error):

// Proof:
const controller = new AbortController();
setTimeout(() => controller.abort('Timeout'), 100);
fetch('...', { signal: controller.signal }).catch(e => {
  console.log(typeof e);  // 'string'
  console.log(e);         // 'Timeout'
});

Suggested Fix

The abort should throw a proper Error:

const timeout = setTimeout(() => {
  controller.abort(new DOMException('Request timed out', 'TimeoutError'));
}, REQUEST_TIMEOUT_MS);

Or use AbortSignal.timeout() (Node 18+) which handles this correctly.

PR #35 addresses the API-side error handling. We'll update it to also fix the CLI timeout issue.

@sergical commented on GitHub (Jan 25, 2026): ## Update: Additional Root Cause Found After further investigation, we found that the "Unauthorized" error was actually masking a **CLI timeout issue**. ### What Actually Happened 1. The delete API was working correctly (verified via direct curl call) 2. The CLI has a 15-second timeout (`REQUEST_TIMEOUT_MS = 15_000`) 3. When the server is slow (cold start?), the timeout fires 4. The CLI throws `controller.abort('Timeout')` — but passing a **string** instead of an Error 5. `pRetry` wraps this in: `Non-error was thrown: "Timeout". You should only throw errors.` 6. This gets caught and displayed, but the confusing error message makes it hard to diagnose ### CLI Bug Location ```javascript // packages/clawdhub/src/http.ts (or dist/http.js) const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS); ``` When `controller.abort()` is called with a string, the AbortSignal throws the string itself (not an Error): ```javascript // Proof: const controller = new AbortController(); setTimeout(() => controller.abort('Timeout'), 100); fetch('...', { signal: controller.signal }).catch(e => { console.log(typeof e); // 'string' console.log(e); // 'Timeout' }); ``` ### Suggested Fix The abort should throw a proper Error: ```javascript const timeout = setTimeout(() => { controller.abort(new DOMException('Request timed out', 'TimeoutError')); }, REQUEST_TIMEOUT_MS); ``` Or use `AbortSignal.timeout()` (Node 18+) which handles this correctly. --- PR #35 addresses the API-side error handling. We'll update it to also fix the CLI timeout issue.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
pull-request
Mirrored from GitHub Pull Request
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: openclaw/clawhub#15
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?