Skip to content

fix(api): return validation errors for malformed skills#112

Open
sriaradhyula wants to merge 1 commit into
cisco-ai-defense:mainfrom
sriaradhyula:fix/api-skill-load-validation
Open

fix(api): return validation errors for malformed skills#112
sriaradhyula wants to merge 1 commit into
cisco-ai-defense:mainfrom
sriaradhyula:fix/api-skill-load-validation

Conversation

@sriaradhyula

@sriaradhyula sriaradhyula commented May 13, 2026

Copy link
Copy Markdown

Summary

  • Map SkillLoadError from the scanner loader to HTTP 422 in the FastAPI router instead of allowing it to fall through to the generic HTTP 500 handler.
  • Redact the resolved skill directory from validation details so upload temp paths are not exposed to API clients.
  • Add a regression test for /scan-upload with malformed SKILL.md metadata, and document the 422 behavior for malformed skill packages.

Problem

Downstream API callers currently cannot distinguish a malformed skill package from an unexpected scanner/server failure. For example, a ZIP containing a SKILL.md with description but no required name field raises SkillLoadError from the loader. The router catches only ValueError before the broad Exception handler, so the client receives:

{\"detail\":\"Internal scan error\"}

That makes UI and CI integrations show opaque server errors even though the user can fix the submitted skill metadata. This came up while integrating cisco-ai-skill-scanner in cnoe-io/ai-platform-engineering and led to a temporary downstream image patch there.

Downstream usage

This is needed by cnoe-io/ai-platform-engineering, which packages cisco-ai-skill-scanner as the skill-scanner service used by its skills UI and catalog flows. In that platform, users can upload or generate Agent Skills from the UI, and the backend sends the packaged skill ZIP to this service /scan-upload endpoint before accepting the skill.

When a generated or uploaded SKILL.md is malformed, such as missing the required name field, CAIPE needs to show actionable validation feedback to the user. With the current upstream behavior, the same user-correctable input error becomes an HTTP 500 and appears as a scanner/server failure. CAIPE currently carries a temporary image-local patch for this behavior; once this fix is released upstream, that downstream patch can be removed and CAIPE can consume the fixed cisco-ai-skill-scanner package directly.

Approach

The change treats loader failures as client validation failures:

  • SkillLoadError now returns HTTP 422 with Invalid skill package: ... detail.
  • ValueError continues to return HTTP 400 for policy/request setup problems.
  • Unexpected exceptions still return HTTP 500 and continue to be logged server-side.
  • The helper replaces the resolved skill directory path with skill directory before returning the detail to avoid leaking upload extraction paths.

Test plan

  • Added failing regression coverage first:
    • uv run pytest tests/test_api_endpoints.py::TestUploadEndpoint::test_upload_malformed_skill_returns_validation_error -q
    • Initially failed with 500 == 422.
  • Verified the targeted regression passes after the fix:
    • uv run pytest tests/test_api_endpoints.py::TestUploadEndpoint::test_upload_malformed_skill_returns_validation_error -q
  • Ran API endpoint suite:
    • uv run pytest tests/test_api_endpoints.py -q
    • 62 passed, 1 skipped
  • Ran full test suite:
    • uv run pytest tests/ -q
    • 1301 passed, 5 skipped
  • Ran lint and pre-commit:
    • uv run ruff check .
    • uv run pre-commit run --all-files

Malformed skill metadata raised SkillLoadError from the scanner and
fell through the API router generic exception handler, producing an
opaque HTTP 500. Map loader validation failures to HTTP 422 so API
clients can surface actionable feedback while preserving generic 500s
for unexpected scanner failures.

Signed-off-by: Sri Aradhyula <sraradhy@cisco.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant