Skip to content

API /v1/bundle double-wraps INVALID_REQUEST as INTERNAL 500 (retryable: true) #679

Description

@lockwobr

Bug Description

POST /v1/bundle returns HTTP 500 / code: INTERNAL / retryable: true
for request-validation failures that are raised inside the deployer
(e.g., path-traversal in a component name). The inner structured error
already carries ErrCodeInvalidRequest, but the outer wrap clobbers it
with ErrCodeInternal. API clients that retry on 5xx or on
retryable: true will hammer the server pointlessly.

Impact

Medium — automated clients retry on 5xx; breaks the API error contract.
Caught during v0.12.0-rc1 smoke testing.

Component

API server (aicrd) + bundler

Regression?

Unknown / first time exercising this error path via the API.

Steps to Reproduce

# Start API
make server &

cat > /tmp/evil-recipe.json <<'JSON'
{
  "kind":"RecipeResult",
  "apiVersion":"aicr.nvidia.com/v1alpha1",
  "componentRefs":[{
    "name":"../evil","namespace":"default","chart":"nginx",
    "type":"Helm","source":"https://example/","version":"1.0.0"
  }],
  "deploymentOrder":["../evil"]
}
JSON

curl -s -X POST 'http://localhost:8080/v1/bundle' \
  -H 'Content-Type: application/json' \
  --data-binary @/tmp/evil-recipe.json \
  -w '\nHTTP %{http_code}\n'

Expected Behavior

HTTP 400 with:

{
  "code": "INVALID_REQUEST",
  "message": "invalid component name \"../evil\": must not contain path separators or parent directory references",
  "retryable": false
}

Consistent with sibling validation failures on the same endpoint (malformed
JSON body, empty recipe, unknown deployer, unknown ?dynamic= component)
which all correctly return 400 INVALID_REQUEST.

Actual Behavior

HTTP 500:

{
  "code": "INTERNAL",
  "message": "failed to generate bundle",
  "details": {"error": "[INVALID_REQUEST] invalid component name \"../evil\": must not contain path separators or parent directory references"},
  "retryable": true
}

Root Cause

pkg/bundler/bundler.go:343 in runDeployer:

output, err := d.Generate(ctx, dir)
if err \!= nil {
    return nil, errors.Wrap(errors.ErrCodeInternal, "failed to generate bundle", err)
}

This wraps every deployer error with ErrCodeInternal regardless of the
inner error's code. The helm deployer's component-name safety check
(pkg/bundler/deployer/helm/helm.go:206-207) returns
errors.New(errors.ErrCodeInvalidRequest, ...) — but the outer wrap
overrides it. Violates the CLAUDE.md rule "Don't double-wrap errors that
already have proper codes."

Proposed Fix

At the runDeployer boundary, preserve the inner structured error's code
instead of unconditionally wrapping with ErrCodeInternal. Either return
the inner error as-is (preferred per CLAUDE.md) or wrap only when the
inner error has no structured code.

Matching sites worth auditing for the same pattern:

  • pkg/bundler/bundler.go:239 (failed to create output directory) — genuinely internal, fine
  • pkg/bundler/bundler.go:247 (failed to extract component values) — check
  • pkg/bundler/bundler.go:262 (failed to copy external data files) — check
  • pkg/bundler/bundler.go:319 (failed to collect component manifests) — check

Environment

  • AICR version: v0.12.0-rc1
  • Install method: build from source
  • Platform: N/A (API server only)

Metadata

Metadata

Assignees

Type

Fields

No fields configured for Bug.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions