feat(api): add scan security verification endpoint and non-suspicious filters

[ngutman]

Summary

Adds API support for skill security scan verification and aligns API filtering with website behavior for hiding suspicious skills.

What Changed

• Added GET /api/v1/skills/{slug}/scan for normalized security scan verification details.
• Added merged VT + LLM security snapshot output for version and scan responses.
• Added nonSuspiciousOnly support to GET /api/v1/search and GET /api/v1/skills.
• Added canonical-over-legacy boolean query precedence and shared parsing helpers.
• Fixed trending list behavior for nonSuspiciousOnly=true to use a dedicated clean leaderboard instead of filtering the first overall entries.
• Restored legacy publish compatibility for omitted acceptLicenseTerms while still rejecting explicit false.
• Restored public owner sanitization on api.skills.getBySlug and re-added regression coverage.
• Updated OpenAPI to include moderation and scan response surfaces.

Validation

• bun run test
• bun x tsc -p packages/schema/tsconfig.json --noEmit
• bun x tsc -p packages/clawdhub/tsconfig.json --noEmit

Replacement for #537 because the original PR head lives on an external fork branch that could not be updated directly.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
clawhub	Ready	Preview, Comment	Mar 13, 2026 0:14am

[greptile-apps]

Greptile Summary

This PR introduces a security scan verification endpoint (GET /api/v1/skills/{slug}/scan), a merged VT+LLM SecuritySnapshot type for version and scan responses, nonSuspiciousOnly query filtering across search and listing APIs, and a dedicated pre-computed non-suspicious trending leaderboard. It also restores public owner sanitization in getBySlug and relaxes the acceptLicenseTerms requirement for legacy CLI compatibility.

Notable changes:

• buildSkillSecuritySnapshot correctly merges VT and LLM results using a priority map, using verdict ?? status so a completed LLM analysis with a valid verdict is never misclassified as error. Tests are comprehensive.
• filterPublicSkillPage + trending double-filter (TRENDING_NON_SUSPICIOUS_LEADERBOARD_KIND lookup + in-loop isSkillSuspicious guard) is a sound defense-in-depth pattern.
• parseBooleanQueryParam / resolveBooleanQueryParam cleanly centralizes query param parsing with legacy alias support.
• hasAcceptedLegacyLicenseTerms (acceptLicenseTerms !== false) is defined identically in both convex/httpApi.ts and convex/httpApiV1/skillsV1.ts. It should be extracted to the new convex/lib/httpUtils.ts to avoid divergence.
• The new publish behavior (omitted acceptLicenseTerms → permitted) has no dedicated test case. The direct Convex publishVersion action still uses the strict !== true check, creating an undocumented divergence between the HTTP API and the action path.
• security is undefined (field absent) in version responses but can be null in scan responses; making these consistent would simplify client handling.

Confidence Score: 4/5

• This PR is safe to merge with minor follow-up recommended for test coverage and code deduplication.
• The core security snapshot logic, leaderboard split, and filter plumbing are all well-tested and correct. The main concerns are non-blocking: a duplicated helper function that creates a drift risk, missing regression tests for the intentional license-terms relaxation, and a minor null/undefined inconsistency in the security field across two endpoint shapes. No correctness bugs were found in the scan endpoint, trending logic, or boolean param parsing.
• convex/httpApiV1/skillsV1.ts and convex/httpApi.ts — specifically the duplicated hasAcceptedLegacyLicenseTerms helper and missing test coverage for the omitted-acceptLicenseTerms publish path.

Comments Outside Diff (2)

1. convex/httpApi.ts, line 184-185 (link)

   Duplicated helper function across files

   hasAcceptedLegacyLicenseTerms is defined identically in both convex/httpApi.ts (lines 184-185) and convex/httpApiV1/skillsV1.ts (lines 874-875). If the policy logic ever needs to change, both copies would need to be updated in sync. This helper should be extracted to convex/lib/httpUtils.ts (which already exists after this PR) to keep it in a single place.

   This also applies to convex/httpApiV1/skillsV1.ts:874-875.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/httpApi.ts
   Line: 184-185
   
   Comment:
   **Duplicated helper function across files**
   
   `hasAcceptedLegacyLicenseTerms` is defined identically in both `convex/httpApi.ts` (lines 184-185) and `convex/httpApiV1/skillsV1.ts` (lines 874-875). If the policy logic ever needs to change, both copies would need to be updated in sync. This helper should be extracted to `convex/lib/httpUtils.ts` (which already exists after this PR) to keep it in a single place.
   
   
   
   This also applies to `convex/httpApiV1/skillsV1.ts:874-875`.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. convex/httpApiV1/skillsV1.ts, line 874-876 (link)

   Missing test coverage for the acceptLicenseTerms omission path

   The function changes behavior from requiring an explicit true to allowing undefined (treating it as accepted). While the PR description describes this as intentional legacy compatibility, neither convex/httpApi.handlers.test.ts nor convex/httpApiV1.handlers.test.ts contain a test case that publishes without providing acceptLicenseTerms at all.

   Without a dedicated test, it's easy for a future refactor (e.g. tightening the check back to !== true) to silently break legacy clients and the regression only surfaces in production. There should also be a test verifying that explicit false is still rejected.

   Additionally, the direct Convex action publishVersion at convex/skills.ts:3880 still uses the strict acceptLicenseTerms !== true check, so the HTTP API and the Convex action now have divergent validation: HTTP allows omission, the action does not. This inconsistency is worth documenting explicitly.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/httpApiV1/skillsV1.ts
   Line: 874-876
   
   Comment:
   **Missing test coverage for the `acceptLicenseTerms` omission path**
   
   The function changes behavior from requiring an explicit `true` to allowing `undefined` (treating it as accepted). While the PR description describes this as intentional legacy compatibility, neither `convex/httpApi.handlers.test.ts` nor `convex/httpApiV1.handlers.test.ts` contain a test case that publishes without providing `acceptLicenseTerms` at all.
   
   Without a dedicated test, it's easy for a future refactor (e.g. tightening the check back to `!== true`) to silently break legacy clients and the regression only surfaces in production. There should also be a test verifying that explicit `false` is still rejected.
   
   Additionally, the direct Convex action `publishVersion` at `convex/skills.ts:3880` still uses the strict `acceptLicenseTerms !== true` check, so the HTTP API and the Convex action now have divergent validation: HTTP allows omission, the action does not. This inconsistency is worth documenting explicitly.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: convex/httpApi.ts
Line: 184-185

Comment:
**Duplicated helper function across files**

`hasAcceptedLegacyLicenseTerms` is defined identically in both `convex/httpApi.ts` (lines 184-185) and `convex/httpApiV1/skillsV1.ts` (lines 874-875). If the policy logic ever needs to change, both copies would need to be updated in sync. This helper should be extracted to `convex/lib/httpUtils.ts` (which already exists after this PR) to keep it in a single place.

```suggestion
// Move to convex/lib/httpUtils.ts and import here
```

This also applies to `convex/httpApiV1/skillsV1.ts:874-875`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: convex/httpApiV1/skillsV1.ts
Line: 874-876

Comment:
**Missing test coverage for the `acceptLicenseTerms` omission path**

The function changes behavior from requiring an explicit `true` to allowing `undefined` (treating it as accepted). While the PR description describes this as intentional legacy compatibility, neither `convex/httpApi.handlers.test.ts` nor `convex/httpApiV1.handlers.test.ts` contain a test case that publishes without providing `acceptLicenseTerms` at all.

Without a dedicated test, it's easy for a future refactor (e.g. tightening the check back to `!== true`) to silently break legacy clients and the regression only surfaces in production. There should also be a test verifying that explicit `false` is still rejected.

Additionally, the direct Convex action `publishVersion` at `convex/skills.ts:3880` still uses the strict `acceptLicenseTerms !== true` check, so the HTTP API and the Convex action now have divergent validation: HTTP allows omission, the action does not. This inconsistency is worth documenting explicitly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: convex/httpApiV1/skillsV1.ts
Line: 706-712

Comment:
**`security` field is `undefined` vs `null` inconsistency**

In the versions detail response, `security` is spread as `security: security ?? undefined`, meaning the field is omitted when there is no scan data. In the scan endpoint response (line ~778), `security` is spread directly and can be `null`.

The OpenAPI schema for `SkillScanResponse` documents `security` as `anyOf: [SecuritySnapshot, null]`, which correctly captures the `null` case. But the versions response uses `undefined` (field absent), whereas the OpenAPI `SkillVersionResponse` schema references `SecuritySnapshot` without `null`. While both work from a consumer perspective, making the field consistently `null` (and always present) across both response shapes would make client-side handling simpler.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: d146a61}

[greptile-apps]

security field is undefined vs null inconsistency

In the versions detail response, security is spread as security: security ?? undefined, meaning the field is omitted when there is no scan data. In the scan endpoint response (line ~778), security is spread directly and can be null.

The OpenAPI schema for SkillScanResponse documents security as anyOf: [SecuritySnapshot, null], which correctly captures the null case. But the versions response uses undefined (field absent), whereas the OpenAPI SkillVersionResponse schema references SecuritySnapshot without null. While both work from a consumer perspective, making the field consistently null (and always present) across both response shapes would make client-side handling simpler.

Prompt To Fix With AI

This is a comment left during a code review.
Path: convex/httpApiV1/skillsV1.ts
Line: 706-712

Comment:
**`security` field is `undefined` vs `null` inconsistency**

In the versions detail response, `security` is spread as `security: security ?? undefined`, meaning the field is omitted when there is no scan data. In the scan endpoint response (line ~778), `security` is spread directly and can be `null`.

The OpenAPI schema for `SkillScanResponse` documents `security` as `anyOf: [SecuritySnapshot, null]`, which correctly captures the `null` case. But the versions response uses `undefined` (field absent), whereas the OpenAPI `SkillVersionResponse` schema references `SecuritySnapshot` without `null`. While both work from a consumer perspective, making the field consistently `null` (and always present) across both response shapes would make client-side handling simpler.

How can I resolve this? If you propose a fix, please make it concise.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f0b422464

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Bound non-suspicious scan to avoid Convex read-limit failures

takeTopNonSuspiciousTrendingEntries performs a ctx.db.get for each candidate entry until it finds limit clean skills, and in rebuildTrendingLeaderboardAction the entries input is built from every skill seen in the 7-day window. In high-churn windows (especially when many top-ranked skills are suspicious/deleted), this loop can read thousands of skills docs in one query and hit Convex documentsReadLimit (32K), causing the non-suspicious leaderboard rebuild to fail and leaving sort=trending&nonSuspiciousOnly=true stale.

Useful? React with 👍 / 👎.