feat: Adding support for providing product code in createEntitlement API

[absarasw]

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description. Or if there's no issue created, make sure you
  describe here the problem you're solving.
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

If the PR is changing the API specification:

• make sure you add a "Not implemented yet" note the endpoint description, if the implementation is not ready
  yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
• make sure you add at least one example of the request and response.

If the PR is changing the API implementation or an entity exposed through the API:

• make sure you update the API specification and the examples to reflect the changes.

If the PR is introducing a new audit type:

• make sure you update the API specification with the type, schema of the audit result and an example

Related Issues

Thanks for contributing!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger a minor release when merged.

[ravverma]

hey @absarasw
Thanks for this taking this up, it was tech debt, it is looking solid , just need some change before merge

PR #1910 Review: Adding productCode Support in createEntitlement API

Summary of Changes

The PR modifies src/controllers/entitlements.js to accept an optional productCode field in the request body (defaulting to 'LLMO'), validates it against known product codes (LLMO, ASO, ACO), and passes the user-supplied product code through to TierClient.createForOrg. Previously, the product code was hardcoded to LLMO.

---

🔴 MUST Fix

1. User-supplied input reflected in error response without sanitization (Security)

The PR constructs an error message that embeds the raw user-supplied productCode directly:

return badRequest(`Invalid product code: ${productCode}. Must be one of: ${[...VALID_PRODUCT_CODES].join(', ')}`);

While JSON responses mitigate HTML injection, a malicious caller could send an extremely long string (e.g. 10MB) as productCode, which would:

• Bloat the error response
• Bloat log output if logged upstream
• Potentially cause memory/performance issues

Recommendation: Truncate or sanitize the reflected value:

const displayCode = String(productCode).substring(0, 50);
return badRequest(`Invalid product code: ${displayCode}. Must be one of: ${[...VALID_PRODUCT_CODES].join(', ')}`);

Or use the existing cleanupHeaderValue() utility the codebase already has for sanitizing values.

---

2. Raw e.message returned to client via internalServerError(e.message) (Info Leaking)

} catch (e) {
  context.log.error(`Error creating entitlement for organization ${organizationId}: ${e.message}`);
  return internalServerError(e.message);
}

This is a pre-existing issue but the PR does not address it and adds more code paths (new product code logic, TierClient calls) that could throw with internal details. The raw error message from downstream services (TierClient, database, etc.) could contain:

• Internal service URLs
• Database connection details
• Stack trace fragments
• Third-party API error details

Recommendation: Return a generic error message to the client and keep the detailed error only in logs:

return internalServerError('Failed to create entitlement');

This applies to both the createEntitlement and getByOrganizationID catch blocks.

---

🔶 SHOULD Fix

1. No input type validation on productCode (Null Check / Security)

The destructuring const { productCode = EntitlementModel.PRODUCT_CODES.LLMO } = context.data || {}; handles undefined and null data, but does not verify that productCode is a string. If a caller sends productCode: 123 or productCode: { "$ne": "LLMO" }, the VALID_PRODUCT_CODES.has() check would still reject it (since the Set contains strings), but the non-string value would be interpolated into the error message via toString().

Recommendation: Add a type check:

if (typeof productCode !== 'string' || !VALID_PRODUCT_CODES.has(productCode)) {
  return badRequest(`Invalid product code. Must be one of: ${[...VALID_PRODUCT_CODES].join(', ')}`);
}

---

2. Missing OpenAPI spec update

Per the project's rules (OpenAPI First principle), changes to API endpoints must include matching OpenAPI spec changes. The POST /organizations/:organizationId/entitlements endpoint now accepts an optional productCode in the request body, but no spec changes are included in this PR.

Recommendation: Update docs/openapi/ to document the new optional productCode request body field and its valid values.

---

3. Route test missing handler assertion

The route test fix (adding createEntitlement: () => null to mockEntitlementController) is correct and necessary, but there is no explicit assertion verifying the route mapping POST /organizations/:organizationId/entitlements maps to entitlementController.createEntitlement (unlike most other routes in the file).

---

✅ Positive Aspects

1. Excellent backward compatibility - The default productCode = EntitlementModel.PRODUCT_CODES.LLMO with context.data || {} fallback ensures existing callers that don't send a body or don't include productCode continue to work identically.
2. Good use of Set for validation - Using new Set(Object.values(EntitlementModel.PRODUCT_CODES)) keeps valid codes in sync with the model definition automatically. No hardcoded strings to maintain separately.
3. Proper admin access check preserved - The hasAdminAccess() guard remains in place and is checked first, before any body parsing.
4. Comprehensive test coverage - The PR adds four new test cases covering: explicit product code (LLMO), default when data is empty {}, default when data is absent, alternative product code (ASO), and invalid product code (verifies 400 + message content).
5. JSDoc correction - Fixed the return type description from "Array of entitlements response" to "Created entitlement response".
6. Follows existing patterns - The implementation is consistent with the controller factory pattern used throughout the codebase.
7. Constant renaming improves clarity - LLMO_TIER to FREE_TRIAL_TIER is more accurate since the tier is not LLMO-specific and is now shared across product codes.

[ravverma]

Added required change in comment above

[Kanishkavijay39]

minor suggestion added, rest looks good as compatibility is maintained.

[ravverma]

Hi @absarasw, Went through the latest commits on the branch — here's the status:

✅ Issue 4 — OpenAPI spec update

Discussed offline — intentionally deferred. No action needed here.

❌ Issue 1 — Input sanitization on productCode (MUST fix)

The value is still reflected unsanitized in the badRequest response:

return badRequest(`Invalid product code: ${productCode}. Must be one of: ...`);

A caller can send an arbitrarily large string and bloat the response/logs. Please truncate before reflecting:

const safeCode = String(productCode).substring(0, 50);
return badRequest(`Invalid product code: ${safeCode}. Must be one of: ${[...VALID_PRODUCT_CODES].join(', ')}`);

❌ Issue 2 — Raw e.message returned to client (MUST fix)

Both catch blocks still return internalServerError(e.message), which can leak internal details (DB errors, service URLs). Please use a generic message:

return internalServerError('Failed to create entitlement');

❌ Issue 3 — Missing type guard on productCode

No typeof check added. A non-string value (e.g. 123, {}) will pass through to .toString() before being reflected. Please add:

if (typeof productCode !== 'string' || !VALID_PRODUCT_CODES.has(productCode)) {
  return badRequest(`Invalid product code. Must be one of: ${[...VALID_PRODUCT_CODES].join(', ')}`);
}

❌ Issue 5 — Route test missing explicit mapping assertion

The createEntitlement: () => null stub was added — thanks for that. But there's still no assertion verifying POST /organizations/:organizationId/entitlements actually maps to entitlementController.createEntitlement, unlike most other routes in the file.

---

Please address issues 1–3 before merge — they are security-relevant. Issue 5 is a nice-to-have. Happy to re-review once updated! 🙌

[ravverma]

Seems from last review, changes are not addressed. please check

[ravverma]

Hi @absarasw, thanks for addressing the feedback so promptly! 🙌 Verified the latest commits on the branch — all outstanding comments are now resolved:

✅ Issue 1 — Input sanitization on productCode

Generic message used — productCode no longer reflected in the response.

✅ Issue 2 — Raw e.message returned to client

Both catch blocks now return generic messages ('Failed to create entitlement' / 'Failed to retrieve entitlements').

✅ Issue 3 — Type guard on productCode

typeof productCode !== 'string' check added alongside the VALID_PRODUCT_CODES.has() guard.

✅ Issue 4 — OpenAPI spec update

Discussed offline — intentionally deferred. No action needed.

✅ Issue 5 — Route test assertions

Explicit assertions added for both GET and POST /organizations/:organizationId/entitlements.

---

LGTM! Approving. 🚀

[solaris007]

🎉 This PR is included in version 1.347.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀