fix(web): hide filesystem path and raw prompt in Skill tool card#455
fix(web): hide filesystem path and raw prompt in Skill tool card#455
Conversation
The Skill tool card was exposing the full absolute filesystem path (including username, plugin cache structure, and version numbers) and the raw SKILL.md prompt content. Register a dedicated Skill presentation in knownTools with a friendly title showing only the skill name, and add a SkillResultView that displays "Skill loaded" instead of the raw output. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run>
Deploying hapi with
|
| Latest commit: |
2ea3edf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://294e06ad.hapi-bqd.pages.dev |
| Branch Preview URL: | https://fix-skill-tool-card-privacy.hapi-bqd.pages.dev |
There was a problem hiding this comment.
Findings
- [Major] Skill input is still exposed in the details dialog. This diff only sanitizes the result pane;
ToolCardstill falls back to the generic input renderer for tools without a dedicated full view, andSkillis not registered there, so opening the card still shows the raw JSON input containing the skill path /SKILL.mdcontent. Evidenceweb/src/components/ToolCard/views/_results.tsx:510,web/src/components/ToolCard/ToolCard.tsx:145,web/src/components/ToolCard/ToolCard.tsx:392
Suggested fix:const SkillView: ToolViewComponent = ({ block }) => { const skillName = getInputStringAny(block.tool.input, ['skill']) return <div className="text-sm text-[var(--app-hint)]">{skillName ? `Skill: ${skillName}` : 'Skill'}</div> } export const toolFullViewRegistry: Record<string, ToolViewComponent> = { ..., Skill: SkillView }
Summary
- Review mode: initial
- 1 major issue found: the PR stops showing the raw result, but the raw Skill input is still visible in the dialog, so the filesystem path /
SKILL.mdcontent leak is not fully fixed. - Residual risk: no regression test covers the sanitized Skill presentation path.
Testing
- Not run (automation; PR code is untrusted in review workflow)
HAPI Bot
| return <ChecklistList items={todos} /> | ||
| } | ||
|
|
||
| const SkillResultView: ToolViewComponent = (props: ToolViewProps) => { |
There was a problem hiding this comment.
[MAJOR] This only sanitizes the result pane. The details dialog still renders the raw tool input for Skill, because ToolCard falls back to renderToolInput(props.block) when there is no dedicated full view, and Skill is not registered in that full-view registry (web/src/components/ToolCard/ToolCard.tsx:145, web/src/components/ToolCard/ToolCard.tsx:392). Opening the card will therefore still expose the absolute skill path / SKILL.md content that this PR is trying to hide.
Suggested fix:
const SkillView: ToolViewComponent = ({ block }) => {
const skillName = getInputStringAny(block.tool.input, ['skill'])
return <div className="text-sm text-[var(--app-hint)]">{skillName ? `Skill: ${skillName}` : 'Skill'}</div>
}
export const toolFullViewRegistry: Record<string, ToolViewComponent> = {
...,
Skill: SkillView
}Add SkillFullView to toolFullViewRegistry so the details dialog shows only the skill name instead of falling back to renderToolInput which would expose raw JSON input. via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run>
There was a problem hiding this comment.
Findings
- [Major] Error states can be mislabeled as successful skill loads.
SkillResultViewonly renders an error whenextractTextFromResult(result)returns text; if the tool returns any non-text/unknown error payload, it falls through toSkill "<name>" loaded, which turns a failed load into a success-looking card. Evidenceweb/src/components/ToolCard/views/_results.tsx:521,web/src/components/ToolCard/views/_results.tsx:529
Suggested fix:if (state === 'error') { const text = extractTextFromResult(result) return ( <div className="text-sm text-red-600"> {text?.trim() ? text : 'Failed to load skill'} </div> ) }
Summary
- Review mode: follow-up after new commits
- 1 major issue found: the new
SkillResultViewcan render failed skill loads asloadedwhenever the error payload is not text-extractable. - Residual risk: no test covers the new
Skillfull-view/result-view behavior for both success and error states.
Testing
- Not run (automation; PR code is untrusted in review workflow)
HAPI Bot
| } | ||
|
|
||
| // For errors, show the error text | ||
| if (state === 'error') { |
There was a problem hiding this comment.
[MAJOR] This error branch only handles text-extractable payloads. If state === "error" but extractTextFromResult(result) returns null, the component falls through to the success path and renders Skill "<name>" loaded instead of an error, so some failed skill loads will look successful.
Suggested fix:
if (state === 'error') {
const text = extractTextFromResult(result)
return (
<div className="text-sm text-red-600">
{text?.trim() ? text : 'Failed to load skill'}
</div>
)
}When the error payload is not text-extractable, fall back to a generic "Failed to load skill" message instead of falling through to the success path showing "Skill loaded". via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run>
There was a problem hiding this comment.
Findings
- None.
Summary
- Review mode: follow-up after new commits
- No additional issues found in the latest PR diff.
- Residual risk: Not found in repo/docs: automated web coverage for the new
Skilltool-card title/full-view/result-view paths.
Testing
- Not run (automation; PR code is untrusted in review workflow)
HAPI Bot
Summary
Skillentry inknownToolswithPuzzleIconand a friendly title (Skill: <name>) instead of falling through to generic renderingSkillResultViewin the result registry that displaysSkill "xxx" loadedfor success and error text for failures, hiding the full filesystem path and raw SKILL.md prompt contentminimal: trueso the card does not expand inline contentCloses #453
Test plan
Skill: <skill-name>with a puzzle iconSkill "xxx" loadedinstead of the raw path and prompt content