fix: activate skills when VT scan is unavailable or stales out#300
Open
superlowburn wants to merge 2 commits intoopenclaw:mainfrom
Open
fix: activate skills when VT scan is unavailable or stales out#300superlowburn wants to merge 2 commits intoopenclaw:mainfrom
superlowburn wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Published skills stay permanently hidden in search when VirusTotal cannot produce a verdict. Three code paths leave moderationStatus as 'hidden' with no recovery: 1. VT_API_KEY not configured — scan skipped, skill stays hidden 2. VT hash not found after 10 poll attempts — marked stale, stays hidden 3. VT hash found but no Code Insight after 10 attempts — same Fix: call setSkillModerationStatusActiveInternal in all three paths so the skill becomes searchable. If VT later returns a malicious verdict, approveSkillByHashInternal will correctly re-hide and flag it. Closes openclaw#139 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
@superlowburn is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
convex/vt.ts
Outdated
Comment on lines
536
to
540
| // Activate the skill so it appears in search — absence of a VT | ||
| // verdict should not permanently hide a published skill. | ||
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | ||
| skillId, | ||
| }) |
There was a problem hiding this comment.
bypasses quality gate quarantine - skills with moderationReason: 'quality.low' will be activated even though they should remain hidden
check the skill's moderationReason before activating:
Suggested change
| // Activate the skill so it appears in search — absence of a VT | |
| // verdict should not permanently hide a published skill. | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId, | |
| }) | |
| // Activate the skill so it appears in search — absence of a VT | |
| // verdict should not permanently hide a published skill. | |
| const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, { skillId }) | |
| if (skill?.moderationReason !== 'quality.low') { | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId, | |
| }) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/vt.ts
Line: 536:540
Comment:
bypasses quality gate quarantine - skills with `moderationReason: 'quality.low'` will be activated even though they should remain hidden
check the skill's `moderationReason` before activating:
```suggestion
// Activate the skill so it appears in search — absence of a VT
// verdict should not permanently hide a published skill.
const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, { skillId })
if (skill?.moderationReason !== 'quality.low') {
await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, {
skillId,
})
}
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment.
Fixed in commit a8e62e2. Added getSkillByIdInternal lookup and check if (skill?.moderationReason !== "quality.low") before activating the skill. This ensures that skills in quality gate quarantine remain hidden even when VT scan is unavailable.
convex/vt.ts
Outdated
Comment on lines
566
to
570
| // Activate the skill so it appears in search — absence of a VT | ||
| // verdict should not permanently hide a published skill. | ||
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | ||
| skillId, | ||
| }) |
There was a problem hiding this comment.
same quality gate bypass issue as above
Suggested change
| // Activate the skill so it appears in search — absence of a VT | |
| // verdict should not permanently hide a published skill. | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId, | |
| }) | |
| // Activate the skill so it appears in search — absence of a VT | |
| // verdict should not permanently hide a published skill. | |
| const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, { skillId }) | |
| if (skill?.moderationReason !== 'quality.low') { | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId, | |
| }) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/vt.ts
Line: 566:570
Comment:
same quality gate bypass issue as above
```suggestion
// Activate the skill so it appears in search — absence of a VT
// verdict should not permanently hide a published skill.
const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, { skillId })
if (skill?.moderationReason !== 'quality.low') {
await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, {
skillId,
})
}
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment.
Fixed in commit a8e62e2. Same fix applied here — added skill lookup and moderationReason check before activating. Skills with quality.low moderation reason will not be activated.
Comment on lines
308
to
317
| console.log('VT_API_KEY not configured, skipping scan — activating skill') | ||
| // Activate the skill so it appears in search despite no VT scan. | ||
| const version = await ctx.runQuery(internal.skills.getVersionByIdInternal, { | ||
| versionId: args.versionId, | ||
| }) | ||
| if (version) { | ||
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | ||
| skillId: version.skillId, | ||
| }) | ||
| } |
There was a problem hiding this comment.
same quality gate bypass - check moderationReason before activating
Suggested change
| console.log('VT_API_KEY not configured, skipping scan — activating skill') | |
| // Activate the skill so it appears in search despite no VT scan. | |
| const version = await ctx.runQuery(internal.skills.getVersionByIdInternal, { | |
| versionId: args.versionId, | |
| }) | |
| if (version) { | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId: version.skillId, | |
| }) | |
| } | |
| console.log('VT_API_KEY not configured, skipping scan — activating skill') | |
| // Activate the skill so it appears in search despite no VT scan. | |
| const version = await ctx.runQuery(internal.skills.getVersionByIdInternal, { | |
| versionId: args.versionId, | |
| }) | |
| if (version) { | |
| const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, { | |
| skillId: version.skillId, | |
| }) | |
| if (skill?.moderationReason !== 'quality.low') { | |
| await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, { | |
| skillId: version.skillId, | |
| }) | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/vt.ts
Line: 308:317
Comment:
same quality gate bypass - check `moderationReason` before activating
```suggestion
console.log('VT_API_KEY not configured, skipping scan — activating skill')
// Activate the skill so it appears in search despite no VT scan.
const version = await ctx.runQuery(internal.skills.getVersionByIdInternal, {
versionId: args.versionId,
})
if (version) {
const skill = await ctx.runQuery(internal.skills.getSkillByIdInternal, {
skillId: version.skillId,
})
if (skill?.moderationReason !== 'quality.low') {
await ctx.runMutation(internal.skills.setSkillModerationStatusActiveInternal, {
skillId: version.skillId,
})
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment.
Fixed in commit a8e62e2. Same fix applied — added skill lookup and moderationReason check before activating. This prevents quality gate quarantine bypass in the non-VT-API path.
Collaborator
|
I rather opt to not show a skill than to show one without scan. We are a target. |
Contributor
Author
|
Thank you for flagging this security concern entirely. You are right that showing skills without a VT scan is too risky given we are a target. I will revise this PR to remove the activation calls entirely. Instead of auto-activating skills when VT is unavailable/stales, we should:
The current fix prioritizes availability over security, which is the wrong tradeoff. I will prepare a revised patch that keeps the quarantine intact and improves observability so publishers understand why their skills aren't appearing. Let me refactor this to address your feedback. |
- Prevent activating skills with quality.low moderation reason - Add skill lookup and moderationReason check in 3 locations where skills are activated - This ensures quality gate quarantine is not bypassed when VT scan is unavailable or stale Resolves review comments on openclaw#300
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #139 — published skills return success but are not visible in search.
Root cause:
insertVersionalways setsmoderationStatus: 'hidden'on publish (skills.ts:3180). The only path to'active'is throughapproveSkillByHashInternal, called when VirusTotal returns a verdict. Three code paths leave the skill permanently hidden with no recovery:VT_API_KEYnot configured —scanWithVirusTotallogs and returns early, skill stays hidden foreverpollPendingScansmarks scan asstalebut never activates the skillSince
toPublicSkill()(public.ts:56) filters out any skill wheremoderationStatus !== 'active', these skills never appear in search results or on the website despite the CLI returning success.Fix: Call
setSkillModerationStatusActiveInternalin all three paths so the skill becomes searchable. If VT later returns a malicious/suspicious verdict,approveSkillByHashInternalwill correctly re-hide and flag it — the existingblocked.malwareflag check intoPublicSkill(line 57) is unaffected.convex/vt.ts— 3 activation calls added (no-API-key path + 2 stale paths)convex/lib/public.test.ts— 5 new tests documenting moderation filtering behaviorTest plan
vitest run)toPublicSkillmoderation filtering: active, hidden, undefined (legacy), soft-deleted, blocked.malwareoxlint --type-aware)🤖 Generated with Claude Code
Greptile Overview
Greptile Summary
This PR fixes a critical issue where published skills would be permanently hidden when VirusTotal scanning fails or times out. The root cause was correctly identified:
insertVersionsetsmoderationStatus: 'hidden'on publish (skills.ts:3180), and without a VT verdict, skills never become searchable.The fix adds three activation calls to make skills visible when VT scanning is unavailable:
VT_API_KEYis not configured (vt.ts:314)The test additions document the
toPublicSkillfiltering behavior for various moderation states, which correctly validates that only'active'skills (or legacy undefined status) appear in search, while hidden, soft-deleted, and malware-blocked skills are filtered out.Critical issue found: The three new activation calls bypass the quality gate quarantine. Skills with
moderationReason: 'quality.low'should remainhiddenuntil manual review (see skills.ts:2335-2344), butsetSkillModerationStatusActiveInternalunconditionally sets status to active. Since quality.low skills are included in the pending scan queue (skills.ts:1661), they will be incorrectly activated when VT stales out.Confidence Score: 2/5
moderationReason: 'quality.low') will be incorrectly activated when VT scans time out, bypassing the manual review requirement enforced inapproveSkillByHashInternal(skills.ts:2335-2344)Last reviewed commit: fb76bdf