fix(suite): fail closed on catalog lifecycle - #314
Draft
davida-ps wants to merge 1 commit into
Draft
Conversation
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Comment on lines
+491
to
+499
| async function discoverCatalog(requestedSkill) { | ||
| const indexUrl = envVar("CLAWSEC_SKILLS_INDEX_URL") || DEFAULT_INDEX_URL; | ||
| const outputIndexUrl = indexUrlForOutput(indexUrl); | ||
| const timeoutMs = parseTimeoutMs(); | ||
| const fallback = await loadFallbackCatalog(); | ||
| const fallbackContext = await loadFallbackCatalog(); | ||
|
|
||
| try { | ||
| const remote = await loadRemoteCatalog(indexUrl, timeoutMs); | ||
| const records = enrichWithFallbackMetadata(remote.records, fallbackContext); |
Contributor
There was a problem hiding this comment.
discoverCatalog() awaits loadFallbackCatalog() before the remote try, so a malformed skills/clawsec-suite/skill.json throws before loadRemoteCatalog() runs and blocks all recommendations — should we wrap loadFallbackCatalog() in its own try/catch, defaulting fallbackContext to [], since the PR description says the suite-local catalog is non-authorizing context?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
skills/clawsec-suite/scripts/discover_skill_catalog.mjs around lines 491-499 inside the
`discoverCatalog(requestedSkill)` function, `fallbackContext` is loaded before the
remote `try` block, so any error thrown by `loadFallbackCatalog()` prevents the remote
catalog fetch from running. Refactor by wrapping the `await loadFallbackCatalog()` call
in its own try/catch, defaulting `fallbackContext` to an empty array (and optionally
storing a warning like "suite-local context unavailable/malformed"), so the subsequent
`loadRemoteCatalog()` attempt proceeds unaffected. Ensure the rest of the function uses
this safe default when fallback parsing fails, and that the remote "available" path
continues to compute recommendations using remoteRecords even when local parsing fails.
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
1 participant
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.
User description
Summary
installable: falserecords from install candidates and expose only scrubbed historical context--skillresolution that fails closed for missing, non-installable, unavailable, or malformed catalog stateclawsec-suitefrom0.1.16to0.1.17Security behavior
Scope
This PR changes only five files under
skills/clawsec-suite/: metadata, documentation, changelog, catalog discovery runtime, and its focused tests.It is intentionally stacked on:
Remote validation
Validated only on
davida@20.14.133.241in a fresh/tmpquarantine after SCP of the reviewed five-file patch and SHA-256 verification:*.test.mjstestsscripts/test-skill-*.mjssimulations with test(release): derive suite simulation version #313 layered inThe validator reported only the existing optional warnings for release-generated advisory checksum artifacts.
No tag, release, publication, or merge is included.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Harden
discover_skill_catalog.mjsto treat the remote catalog as a denial overlay, resolve exact--skillrequests, and fail closed on malformed, unavailable, or non-installable lifecycle data. Updateskill.json,SKILL.md,CHANGELOG.md, and the focused tests to describe the new non-authorizing fallback context, version0.1.17, and guarded install flow.installable: falseand malformed records, and emit install commands only for lifecycle-screened remote candidates.Modified files (2)
Latest Contributors(2)
0.1.17bump and document the new denial-overlay, non-authorizing catalog behavior.Modified files (3)
Latest Contributors(2)