collections-lite update

[streude]

No description provided.

[service-coreservices]

Checkmarx One – Scan Summary & Details – fa9644d5-6a54-4b49-8cf0-9cf61fa93e37

---

New Issues (8)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		Client_DOM_Code_Injection	index.mjs: 289	details Method Lambda at line 289 of /index.mjs gets user input for the sessionId element. The input flows to releaseRefreshLock method, at line 480 of /... Attack Vector
2		Reflected_XSS	index.mjs: 279	details The method Lambda embeds untrusted data in generated output with end, at line 348 of /index.mjs. This untrusted data is embedded into the output ... Attack Vector
3		Reflected_XSS	index.mjs: 274	details The method Lambda embeds untrusted data in generated output with end, at line 348 of /index.mjs. This untrusted data is embedded into the output ... Attack Vector
4		Reflected_XSS	index.mjs: 274	details The method Lambda embeds untrusted data in generated output with end, at line 348 of /index.mjs. This untrusted data is embedded into the output ... Attack Vector
5		Reflected_XSS	index.mjs: 318	details The method Lambda embeds untrusted data in generated output with end, at line 348 of /index.mjs. This untrusted data is embedded into the output ... Attack Vector
6		Open_Redirect	index.mjs: 145	details The potentially tainted value provided by accessToken in /index.mjs at line 145 is used as a destination URL by redirect in /index.mjs at line 15... Attack Vector
7		Open_Redirect	index.mjs: 129	details The potentially tainted value provided by redirect_uri in /index.mjs at line 129 is used as a destination URL by redirect in /index.mjs at line ... Attack Vector
8		Log_Forging	index.mjs: 279	details Method Lambda at line 279 of /index.mjs gets user input from element method. This element’s value flows through the code without being properly s... Attack Vector

---

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	Client_DOM_Open_Redirect	token.html: 702

---

Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

[Copilot]

Pull request overview

Updates the sample UI to use the paginated collections-lite API with Personal and Shared libraries, lazy loading, refreshed navigation, and responsive styling.

Changes:

• Added paginated and lazy-loaded collection trees.
• Improved navigation, search progress UI, and responsive layouts.
• Updated documentation and application structure.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Review summary
token.html	Critical XSS issue from interpolating API-controlled collection names into innerHTML (1 vote); moderate font-count calculation issue (2 votes).
README.md	Documentation updates reviewed.
app.js	Critical XSS issue from interpolating API-controlled collection names into innerHTML (1 vote); moderate font-count calculation issue (2 votes).
app.html	Application structure and accessibility updates reviewed.
app.css	Layout, collection tree, and responsive styling updates reviewed.

Suppressed comments (6)

app.js:223

• The root objects returned by collections-lite are kept in collections, but the Discover view still computes “Total Fonts” by summing col.fontCount. These lite objects expose itemCount for unopened assets, so a normal non-empty response will make that summary show 0 even though the Personal/Shared trees contain assets. Compute the total from a real font-variation count or remove/rename this summary until that count is available.

        collections = [...collectionTrees.personal, ...collectionTrees.shared];

app.js:394

• If loading a collection's children fails, toggleSubFolders() catches the error and returns, but this handler ignores that outcome and immediately calls showCollection() with no loaded children. The click then replaces the error state with a misleading 0-child/0-font details view; return a success/failure result (or rethrow) and only show the details after a successful load.

            await toggleSubFolders(itemContainer, collection);
        }

app.js:401

• getDisplayedChildCount intentionally returns null when the API omits itemCount, but this value is passed as fontCount and showCollection renders it with || 0. That turns an unknown count into a displayed zero (including after a failed lazy-load attempt), which is misleading. Preserve the unknown state or use a separate child-count field/label instead of coercing it to zero.

            fontCount: getDisplayedChildCount(collection),

token.html:916

• The root objects returned by collections-lite are kept in collections, but the Discover view still computes “Total Fonts” by summing col.fontCount. These lite objects expose itemCount for unopened assets, so a normal non-empty response will make that summary show 0 even though the Personal/Shared trees contain assets. Compute the total from a real font-variation count or remove/rename this summary until that count is available.

                collections = [...collectionTrees.personal, ...collectionTrees.shared];

token.html:1086

• If loading a collection's children fails, toggleSubFolders() catches the error and returns, but this handler ignores that outcome and immediately calls showCollection() with no loaded children. The click then replaces the error state with a misleading 0-child/0-font details view; return a success/failure result (or rethrow) and only show the details after a successful load.

                if (hasSubItems) {
                    await toggleSubFolders(itemContainer, collection);

token.html:1094

• getDisplayedChildCount intentionally returns null when the API omits itemCount, but this value is passed as fontCount and showCollection renders it with || 0. That turns an unknown count into a displayed zero (including after a failed lazy-load attempt), which is misleading. Preserve the unknown state or use a separate child-count field/label instead of coercing it to zero.

                    fontCount: getDisplayedChildCount(collection),

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Suppressed comments (10)

README.md:13

• The README still describes token expiry as “JWT-based” in the feature and token-management sections, but this change removes jsonwebtoken and the implementation now derives Redis TTL from the OAuth response's expires_in (and checks Redis TTL). Update those descriptions so the documented refresh behavior matches the new implementation.

- **Paginated and Lazy Loading**: Fetches every root page and loads folder, font set, web project, and digital ad contents when opened

app.js:401

• getDisplayedChildCount() returns the API's itemCount before loading and the number of direct child assets after loading, but this assigns that value to fontCount. For a Folder, those children can be FontSets, WebProjects, or DigitalAds rather than font variations, so the details panel will report an asset count as “Font Count” and describe it as font variations. Keep the child count separate or compute the variation count before populating this field.

            fontCount: getDisplayedChildCount(collection),

app.js:401

• The ...collection spread comes after the normalized id, name, and fontCount values, so it overwrites them. collections-lite assets are represented with assetId/itemCount; for those, showCollection gets an undefined ID/count, causing the details to show the wrong values and the active-item lookup to fail. Spread the raw object first, then apply these fallbacks.

            fontCount: getDisplayedChildCount(collection),

index.mjs:45

• The allowlist omits application/vnd.ms-opentype, a standard content type for OpenType font downloads. Because download paths require hasDownloadContentType, a valid 2xx response with this type is converted into a 502 instead of being delivered.

    'application/font-sfnt',
    'application/vnd.ms-fontobject',

index.mjs:484

• This replaces JWT exp inspection with expires_in/TTL handling, but the README still describes expiration as “JWT-based” in the feature and token-management sections. Please update those statements so operators are not misled about the token lifetime source and fallback behavior.

function getTokenLifetime(value) {
    const lifetime = Number(value);
    return Number.isFinite(lifetime) && lifetime > 0 ? Math.max(1, Math.floor(lifetime)) : 3600;

token.html:1094

• getDisplayedChildCount() returns the API's itemCount before loading and the number of direct child assets after loading, but this assigns that value to fontCount. For a Folder, those children can be FontSets, WebProjects, or DigitalAds rather than font variations, so the details panel will report an asset count as “Font Count” and describe it as font variations. Keep the child count separate or compute the variation count before populating this field.
  token.html:1530
• Search results now live in #search-results-list, but logout only empties #folders-list; setBrowseFontsVisible(false) merely hides the search-results node. After a new user authenticates in the same page, clicking Discover Fonts will reveal the previous user's font results (and IDs). Clear this container on logout and cancel or ignore any in-flight search response so it cannot repopulate the old results after the reset.
  token.html:916
• The new collections-lite objects are normalized with itemCount, but this assignment never derives the fontCount field that the Discover view still sums (collections.reduce(... col.fontCount ...)). With the documented response shape, the “Total Fonts” card therefore renders 0 even when collections contain items. Compute that summary from the new hierarchy/count field, or change the card to a metric that can be known before lazy-loading children.
  token.html:1094
• The ...collection spread comes after the normalized id, name, and fontCount values, so it overwrites them. collections-lite assets are represented with assetId/itemCount; for those, showCollection gets an undefined ID/count, causing the details to show the wrong values and the active-item lookup to fail. Spread the raw object first, then apply these fallbacks.
  app.js:223
• The new collections-lite objects are normalized with itemCount, but this assignment never derives the fontCount field that the Discover view still sums (collections.reduce(... col.fontCount ...)). With the documented response shape, the “Total Fonts” card therefore renders 0 even when collections contain items. Compute that summary from the new hierarchy/count field, or change the card to a metric that can be known before lazy-loading children.

        collections = [...collectionTrees.personal, ...collectionTrees.shared];

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Suppressed comments (9)

README.md:12

• The feature list still describes token expiry detection as JWT-based, but this change removes jsonwebtoken and the implementation now relies on Redis TTLs. Update that description so the README does not document a mechanism the server no longer uses.

- **Library Browser**: Separate Personal and Shared asset trees loaded from `/v1/fontslibrary/collections-lite`

app.js:540

• Child collection requests use the same session-expiration error, but this catch only shows a status message and leaves the authenticated application visible. After the access/refresh token has expired, opening a lazy-loaded asset therefore cannot take the user back to the reconnect flow; handle the session-expired case like loadCollections() does before showing the generic child-load error.

                } catch (error) {
                    console.error(`Failed to load ${collection.name} contents:`, error);
                    showStatus(`Failed to load ${collection.name} contents: ${error.message}`, 'error');
                    return false;

app.js:257

• The proxy returns 401 responses with error: "Authentication required" (see index.mjs), but this check only recognizes Not authenticated and AUTHENTICATION_FAILED. When a refresh token is missing or invalid, the error therefore falls through as a generic load failure, so loadCollections() never takes its session-expired path and the login button keeps retrying collection loading instead of starting OAuth again. Treat the proxy's 401 status/authentication error as session expiration here (or propagate a stable error code).

            const errorTitle = result.error?.title || result.error;
            if (errorTitle === 'Not authenticated' || errorTitle === 'AUTHENTICATION_FAILED') {
                throw new Error('Session expired. Please log in again.');
            }
            throw new Error(result.message || result.error?.detail || `HTTP ${response.status}: Failed to load collections`);

app.js:526

• This lazy-load request can outlive logout. If the user expands an asset and logs out before this await resolves, the handler still mutates the old collection and the click handler then calls showCollection; if a new session is opened before it finishes, stale children/details can be rendered into that session. Track a session/logout generation or abort and ignore child-load results from an obsolete session.

                    collection.children = await fetchCollectionPages(collection.accessType, collection);
                    collection._childrenLoaded = true;
                    renderCollectionChildren(collection, subFoldersContainer);

app.js:444

• These collection rows are clickable div elements, but they have no focusability, button/tree semantics, or keyboard activation handler. Keyboard and assistive-technology users therefore cannot expand or select the new My Library assets. Use native buttons/tree items or add equivalent focus, role, and Enter/Space handling.

    const folderItem = document.createElement('div');
    folderItem.className = isSubItem ? 'sub-folder-item' : 'folder-item';
    if (hasSubItems) {
        folderItem.classList.add('has-children');
    }

    folderItem.dataset.collectionId = collectionId;

app.js:379

• Font selection is also implemented as a click-only div (fontLink), so keyboard users cannot activate fonts after expanding a collection or viewing search results. Give these links native interactive semantics or add focusability and keyboard activation consistently with the collection rows.

    const fontIcon = document.createElement('span');
    fontIcon.className = 'icon';
    fontIcon.textContent = '📝'; // Font icon

    const fontName = document.createElement('span');
    fontName.className = 'name';
    fontName.textContent = font.name || font.displayName || `Font ${index + 1}`;

app.js:539

• The renderer explicitly supports assets that expose only displayName, but the failure path still uses collection.name, producing messages such as Failed to load undefined contents for those assets. Use the same display-name fallback here so a failed lazy-load tells the user which collection failed.

                    console.error(`Failed to load ${collection.name} contents:`, error);
                    showStatus(`Failed to load ${collection.name} contents: ${error.message}`, 'error');

package.json:11

• Removing jsonwebtoken changes expiration handling to Redis TTLs, but the README still describes automatic refresh and token storage as JWT-based (README.md lines 9 and 131). Update those statements so operators are not told that the server decodes JWT expiration claims when the implementation no longer does.

    "express-session": "^1.18.2",

app.js:1296

• Contextual search only submits the query and does not depend on the filter lookup response, but this keeps its button disabled until all three unrelated filter arrays load. If filterslookup fails or omits one optional list, contextual search remains unusable even though its API is available. Enable this button independently and gate only the filtered-font search on loadedCount === 3.

        contextualSearchBtn.disabled = true;