Skip to content

[CatalogDetail] Sanitize snapshot URLs and guard non-array technologies#1694

Open
ritzorama wants to merge 2 commits into
masterfrom
claude/my-designs-bugs-refactor-khpice
Open

[CatalogDetail] Sanitize snapshot URLs and guard non-array technologies#1694
ritzorama wants to merge 2 commits into
masterfrom
claude/my-designs-bugs-refactor-khpice

Conversation

@ritzorama

Copy link
Copy Markdown
Contributor

Notes for Reviewers

Supports the catalog design detail ("My Designs") fixes in layer5io/meshery-cloud.

  • Adds sanitizeCatalogImageUrl and applies it in CatalogCardDesignLogo. Legacy catalog records stored the design snapshot URL wrapped in stray % delimiters, which rendered as a path relative to the app origin and returned HTTP 400. The helper strips the stray delimiters (preserving genuine %XX percent-encoding), requires an absolute http(s) URL, and otherwise falls back to the placeholder icon so no broken request is issued. It is exported from the custom barrel for downstream reuse.
  • Guards TechnologySection against a non-array compatibility so a legacy scalar can never reach .map and crash the design detail page.

Verification: new sanitizeCatalogImageUrl unit test added; the full jest suite (323 tests) passes and tsc introduces no new errors.

Signed commits

  • Yes, I signed my commits.

Generated by Claude Code

…logies

- Add `sanitizeCatalogImageUrl` and apply it in `CatalogCardDesignLogo`:
  legacy catalog records stored the design snapshot URL wrapped in stray '%'
  delimiters, which rendered as a path relative to the app origin and 400'd.
  Strip the stray delimiters (preserving genuine %XX encoding), require an
  absolute http(s) URL, and otherwise fall back to the placeholder icon so no
  broken request is issued.
- Guard `TechnologySection` against a non-array `compatibility` so a legacy
  scalar can never reach `.map` and crash the design detail page.

Exports `sanitizeCatalogImageUrl` from the custom barrel and adds a unit test.

Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
Copilot AI review requested due to automatic review settings July 7, 2026 22:56

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a sanitizeCatalogImageUrl helper function along with unit tests to safely normalize legacy catalog image URLs by stripping stray '%' delimiters. It also adds defensive checks in TechnologySection to prevent crashes when technologies is not an array. Feedback was provided to improve the defensive check in TechnologySection by wrapping scalar strings in an array instead of discarding them, and to correct a typo in the accompanying comment.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +23 to +26
// `compatibility` is an array per the @meshery/schemas contract, but legacy
// records can surface a scalar; guard so a non-array never reaches `.map`
// (which would crash the design detail page).
const technologyList = Array.isArray(technologies) ? technologies : [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The comment mentions compatibility but the prop and variable being handled is technologies. Additionally, if technologies is a legacy scalar string (e.g., "kubernetes"), falling back to an empty array [] will silently discard the technology and show an empty state.

To support legacy scalar strings and defensively guard against null, undefined, empty strings, or non-string values (which would cause a runtime crash when calling tech.toLowerCase()), we can normalize and filter the list to ensure it only contains valid, non-empty strings.

    // technologies is an array per the @meshery/schemas contract, but legacy
    // records can surface a scalar; guard so a non-array never reaches .map
    // (which would crash the design detail page) and wrap scalar strings in an array.
    const technologyList = (Array.isArray(technologies) ? technologies : [technologies]).filter(
      (tech): tech is string => typeof tech === 'string' && tech.trim() !== ''
    );

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves resilience of Sistent’s catalog detail UI when rendering legacy catalog records by sanitizing malformed snapshot URLs before passing them to <img src> and by preventing scalar compatibility values from crashing the technology list rendering.

Changes:

  • Added sanitizeCatalogImageUrl helper to normalize legacy snapshot URLs and avoid issuing broken relative-origin image requests.
  • Updated CatalogCardDesignLogo to use the sanitized URL (or fall back to the placeholder icon).
  • Guarded TechnologySection so non-array technology values can’t reach .map, and added unit tests for the URL sanitizer.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/custom/index.tsx Re-exports sanitizeCatalogImageUrl from the custom barrel for downstream usage.
src/custom/CustomCatalog/index.tsx Re-exports the sanitizer from the CustomCatalog barrel.
src/custom/CustomCatalog/Helper.ts Introduces sanitizeCatalogImageUrl implementation.
src/custom/CustomCatalog/CatalogCardDesignLogo.tsx Uses the sanitizer to avoid broken image requests and fall back cleanly.
src/custom/CatalogDetail/TechnologySection.tsx Guards .map against legacy scalar compatibility values.
src/testing/sanitizeCatalogImageUrl.test.ts Adds unit tests covering sanitizer behavior and edge cases.

Comment on lines +38 to +42
// Guard against legacy/malformed snapshot URLs (e.g. stray '%' delimiters or
// non-absolute values) so we never issue a broken request against the app
// origin — fall back to the placeholder icon instead.
const resolvedSrc = sanitizeCatalogImageUrl(imgURL?.[0]);

Comment on lines +23 to +26
// `compatibility` is an array per the @meshery/schemas contract, but legacy
// records can surface a scalar; guard so a non-array never reaches `.map`
// (which would crash the design detail page).
const technologyList = Array.isArray(technologies) ? technologies : [];
… error state

Addresses review feedback:
- TechnologySection: wrap a legacy scalar `technologies` value in an array
  (rather than discarding it) and keep only non-empty strings, so a scalar,
  null, or non-string value can never reach `.map`/`tech.toLowerCase()`. Also
  corrects the inline comment to reference the `technologies` prop.
- CatalogCardDesignLogo: reset `imgError` when the resolved snapshot src
  changes, so a new valid image is not hidden by a prior load failure.

Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
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.

3 participants