Repository navigation
Add self-service API key usage status - #2908
diegosouzapw merged 3 commits into
Conversation
The tokenCacheKey() SHA-256 digest is an in-memory cache key derived from the session token, not password-at-rest storage. Document why CWE-916 KDFs (bcrypt/scrypt/Argon2) are inapplicable here so the CodeQL js/insufficient-password-hash finding (code-scanning alert 261) is correctly understood as a false positive.
There was a problem hiding this comment.
Code Review
This pull request implements self-service API key usage and quota visibility, introducing the GET /api/v1/me/status endpoint, new self:usage and self:account-quota scopes, a database migration to backfill the usage scope for existing keys, and UI controls in the API Manager. It also hardens the /api/usage/budget endpoint with management authentication and raises the scope validation limit to 32. The review feedback highlights a critical bug where toNumber conversions on ISO strings or Date objects in the self-service status builder will fail and discard the actual budget period, suggesting direct passing to isoOrNull instead. Additionally, it recommends extending isoOrNull to support Date instances and adding explicit type="button" attributes to the new buttons in PermissionsModal to prevent accidental form submissions.
| const periodStartAt = hasBudget | ||
| ? toNumber(summary.periodStartAt, fallbackWindow.periodStartAt) | ||
| : fallbackWindow.periodStartAt; | ||
| const resetAt = hasBudget | ||
| ? toNumber(summary.nextResetAt ?? summary.budgetResetAt, fallbackWindow.resetAt) | ||
| : fallbackWindow.resetAt; |
There was a problem hiding this comment.
Using toNumber on summary.periodStartAt and summary.nextResetAt / summary.budgetResetAt will fail if these fields are returned as ISO strings (e.g., '2026-05-15T12:34:56.000Z') or Date objects. Number('2026-05-15T12:34:56.000Z') evaluates to NaN, causing toNumber to always return the fallback window. This completely discards the actual budget period start and reset dates. Instead, pass the values directly and let isoOrNull handle the type normalization.
| const periodStartAt = hasBudget | |
| ? toNumber(summary.periodStartAt, fallbackWindow.periodStartAt) | |
| : fallbackWindow.periodStartAt; | |
| const resetAt = hasBudget | |
| ? toNumber(summary.nextResetAt ?? summary.budgetResetAt, fallbackWindow.resetAt) | |
| : fallbackWindow.resetAt; | |
| const periodStartAt = hasBudget && summary.periodStartAt != null | |
| ? summary.periodStartAt | |
| : fallbackWindow.periodStartAt; | |
| const resetAt = hasBudget && (summary.nextResetAt ?? summary.budgetResetAt) != null | |
| ? (summary.nextResetAt ?? summary.budgetResetAt) | |
| : fallbackWindow.resetAt; |
| function isoOrNull(value: number | string | null | undefined): string | null { | ||
| if (typeof value === "number" && Number.isFinite(value) && value > 0) { | ||
| return new Date(value).toISOString(); | ||
| } | ||
| if (typeof value === "string" && value.trim()) { | ||
| const parsed = Date.parse(value); | ||
| return Number.isFinite(parsed) ? new Date(parsed).toISOString() : null; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The isoOrNull helper currently only handles number and string types. If summary.periodStartAt or other date fields are returned as Date objects from the database/domain layer, isoOrNull will return null. Extending it to support Date instances makes it more robust.
function isoOrNull(value: number | string | Date | null | undefined): string | null {
if (value instanceof Date) {
return value.toISOString();
}
if (typeof value === "number" && Number.isFinite(value) && value > 0) {
return new Date(value).toISOString();
}
if (typeof value === "string" && value.trim()) {
const parsed = Date.parse(value);
return Number.isFinite(parsed) ? new Date(parsed).toISOString() : null;
}
return null;
}| <button | ||
| role="switch" | ||
| aria-checked={selfUsageEnabled} | ||
| onClick={() => | ||
| setSelfUsageEnabled((prev) => { | ||
| if (prev) setSelfAccountQuotaEnabled(false); | ||
| return !prev; | ||
| }) | ||
| } | ||
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | ||
| selfUsageEnabled | ||
| ? "bg-emerald-500/15 text-emerald-700 dark:text-emerald-300 border border-emerald-500/30" | ||
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | ||
| }`} | ||
| > |
There was a problem hiding this comment.
The button is missing an explicit type="button" attribute. In HTML/React, buttons inside a <form> default to type="submit". If PermissionsModal is wrapped in a form, clicking this button will trigger an accidental form submission instead of just toggling the state.
| <button | |
| role="switch" | |
| aria-checked={selfUsageEnabled} | |
| onClick={() => | |
| setSelfUsageEnabled((prev) => { | |
| if (prev) setSelfAccountQuotaEnabled(false); | |
| return !prev; | |
| }) | |
| } | |
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | |
| selfUsageEnabled | |
| ? "bg-emerald-500/15 text-emerald-700 dark:text-emerald-300 border border-emerald-500/30" | |
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | |
| }`} | |
| > | |
| <button | |
| type="button" | |
| role="switch" | |
| aria-checked={selfUsageEnabled} | |
| onClick={() => | |
| setSelfUsageEnabled((prev) => { | |
| if (prev) setSelfAccountQuotaEnabled(false); | |
| return !prev; | |
| }) | |
| } | |
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | |
| selfUsageEnabled | |
| ? "bg-emerald-500/15 text-emerald-700 dark:text-emerald-300 border border-emerald-500/30" | |
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | |
| }`} | |
| > |
| <button | ||
| role="switch" | ||
| aria-checked={selfAccountQuotaEnabled} | ||
| disabled={!selfUsageEnabled} | ||
| onClick={() => setSelfAccountQuotaEnabled((prev) => !prev)} | ||
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | ||
| selfAccountQuotaEnabled | ||
| ? "bg-amber-500/15 text-amber-700 dark:text-amber-300 border border-amber-500/30" | ||
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | ||
| } ${!selfUsageEnabled ? "opacity-50 cursor-not-allowed" : ""}`} | ||
| > |
There was a problem hiding this comment.
The button is missing an explicit type="button" attribute. Adding type="button" prevents accidental form submissions if the modal is wrapped in a form.
| <button | |
| role="switch" | |
| aria-checked={selfAccountQuotaEnabled} | |
| disabled={!selfUsageEnabled} | |
| onClick={() => setSelfAccountQuotaEnabled((prev) => !prev)} | |
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | |
| selfAccountQuotaEnabled | |
| ? "bg-amber-500/15 text-amber-700 dark:text-amber-300 border border-amber-500/30" | |
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | |
| } ${!selfUsageEnabled ? "opacity-50 cursor-not-allowed" : ""}`} | |
| > | |
| <button | |
| type="button" | |
| role="switch" | |
| aria-checked={selfAccountQuotaEnabled} | |
| disabled={!selfUsageEnabled} | |
| onClick={() => setSelfAccountQuotaEnabled((prev) => !prev)} | |
| className={`inline-flex items-center gap-1.5 px-2.5 py-1.5 rounded-md text-xs font-semibold transition-colors ${ | |
| selfAccountQuotaEnabled | |
| ? "bg-amber-500/15 text-amber-700 dark:text-amber-300 border border-amber-500/30" | |
| : "bg-black/5 dark:bg-white/5 text-text-muted border border-border" | |
| } ${!selfUsageEnabled ? "opacity-50 cursor-not-allowed" : ""}`} | |
| > |
…icts, and fix node:sqlite for Node v20 compatibility
a15750d
into
diegosouzapw:release/v3.8.6
|
Thank you @guanbear for your contribution! This has been successfully merged into the release branch (with migration renumbered to 075 to resolve conflicts, and test compatibility fixes for Node.js v20) and will be included in the next release. |
Integrated into release/v3.8.6
|
Thank you for reviewing and merging this into release/v3.8.6. I appreciate the migration renumbering and the Node.js v20 compatibility fixes. I also reviewed the automated feedback after merge. I will prepare a small follow-up PR against the release branch to address the timestamp normalization/button type/i18n polish items, and to extend self-service account quota visibility from the single Codex connection case to all allowed provider-limit connections while keeping the same opt-in permission model. Thanks again for the quick review and merge. |
|
Follow-up PR opened here: #2931. It addresses the timestamp normalization/button type/i18n polish items from the review and extends quota visibility to all allowed provider-limit connections while keeping the same opt-in permission model. |
Integrated into release/v3.8.6
Integrated into release/v3.8.6
Hotfixes da release/v3.8.7 (perf RAM diegosouzapw#2903, self-service diegosouzapw#2908, analytics diegosouzapw#2904, bump 3.8.7, docs) na main consolidada com 3.8.6.
Integrated into release/v3.8.6
Integrated into release/v3.8.6
Hotfixes da release/v3.8.7 (perf RAM diegosouzapw#2903, self-service diegosouzapw#2908, analytics diegosouzapw#2904, bump 3.8.7, docs) na main consolidada com 3.8.6.
Integrated into release/v3.8.6
Integrated into release/v3.8.6
Hotfixes da release/v3.8.7 (perf RAM diegosouzapw#2903, self-service diegosouzapw#2908, analytics diegosouzapw#2904, bump 3.8.7, docs) na main consolidada com 3.8.6.
Summary
GET /api/v1/me/statusso a delegated API key can view only its own USD usage, budget percentage, token totals, and optional shared Codex account quota.self:usageis enabled by default for new and legacy keys, whileself:account-quotais opt-in and only useful with an explicit single connection./api/usage/budgetwith handler-level management auth.Test Plan
DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx --test tests/unit/api-manager-scope-preservation.test.ts tests/unit/api-key-scope-validation.test.ts tests/unit/api-key-self-service.test.ts tests/unit/api/v1-me-status-route.test.ts tests/unit/budget-route-auth.test.tsnpm run typecheck:corenpm run lint -- --quietnpm run i18n:sync-ui:dry && npm run i18n:check-ui-coveragenpm run build073applied, missing Bearer returned401, default self-service key returned200with own cost/token usage and noaccountQuota, quota-enabled key returned200with Codex session/weekly quota percentages.Notes
npm testwas attempted but did not complete in this local environment; it was manually interrupted after several minutes with no final TAP summary while still in the existing broad unit suite. The focused regression tests above passed.