Add Admin API to the backend + more docs and badges#1
Conversation
…hed requests Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
oh god i forgot to do this for months on my sandbox VM wtfbbq? anyways let sync it out to github then Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Also updated the secrets behind the scenes rn, alongside some OpenAPI metadata chaos updates. Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
📝 WalkthroughWalkthroughAdds GitHub token configuration and admin GitHub-auth endpoints, introduces token validation and caching, refactors logo/icon resolution to remote fetch + KV cache, expands badge endpoints and styles validation, and updates docs, tooling, and editor settings across the repo. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant GitHub
participant KV
Client->>API: Request protected admin endpoint (Bearer token)
API->>KV: Check cached token hash
alt Cache miss or expired
API->>GitHub: Validate token / fetch user & team membership
GitHub-->>API: User data + membership
API->>KV: Store user data with TTL
end
API-->>Client: Return auth result (allowed / denied)
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai configuration |
CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: true
enable_free_tier: true
reviews:
profile: chill
request_changes_workflow: true
high_level_summary: true
high_level_summary_placeholder: '@coderabbitai summary'
auto_title_placeholder: '@coderabbitai'
review_status: true
commit_status: true
poem: true
collapse_walkthrough: true
sequence_diagrams: true
changed_files_summary: true
labeling_instructions: []
path_filters: []
path_instructions: []
abort_on_close: true
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
tools:
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
chat:
auto_reply: true
knowledge_base:
opt_out: false
learnings:
scope: auto
issues:
scope: auto
jira:
project_keys: []
linear:
team_keys: []
pull_requests:
scope: auto
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 42
🧹 Outside diff range and nitpick comments (30)
utils/types.ts (1)
Line range hint
2-12: LGTM! Consider enhancing the documentation.The formatting changes look good. However, since this interface is copied from badge-maker, it would be helpful to add:
- A link to the original source
- A brief description of each property's purpose
Example documentation improvement:
/** * Badge format configuration interface * @see https://github.com/badges/shields/blob/master/badge-maker/types.ts * * @property label - Optional text shown on the left side of the badge * @property message - Required text shown on the right side of the badge * @property labelColor - Optional color for the left side (label) * @property color - Optional color for the right side (message) * @property style - Optional badge style preset * @property logoBase64 - Optional base64-encoded logo image * @property links - Optional array of URLs for left and right side click actions */docs/README.md (1)
3-6: Consider adding Admin API documentation.Given that this PR introduces Admin API functionality to the backend, consider adding a dedicated section or link for Admin API documentation to help developers understand the new administrative capabilities.
Example addition:
- [API Docs, as generated by chanfana and our OpenAPI spec](https://badges.api.lorebooks.wiki/docs) - [API Terms and Privacy](./api-terms.md) - [(unofficial) Hack Club badges](./hackclub-badges.md) - [Available logos and badge logo admin](./logos.md) +- [Admin API Documentation](./admin-api.md).vscode/settings.json (1)
18-21: Consider documenting the rationale for editor settings.While the editor settings are valid, it would be helpful to document why these specific limits and formatting behaviors were chosen, especially the high
colorDecoratorsLimitvalue of 50000.Consider adding a JSON comment block above these settings explaining the rationale:
"conventionalCommits.emojiFormat": "code", + // Increased color decorator limit to handle large files "editor.colorDecoratorsLimit": 50000, + // Enable automatic formatting for consistency "editor.formatOnPaste": true, "editor.formatOnSave": true, "editor.formatOnType": false,lib/utils.ts (1)
2-7: Enhance JSDoc documentation with TypeScript annotations.The documentation should include type information and be more specific about the return value.
/** * Validate badge style name received from the HTTP request before * rendering into makeBadge function. - * @param style String of a style name to validate against - * @returns Either the lowercased style name or the defaults + * @param {string} [style] - Badge style name to validate + * @returns {string} The validated style name (lowercase) or "flat" as default */utils/kv-keys-debugger.ts (1)
Line range hint
1-19: Consider adding error handling and type safety.As this is a debugging utility, it would be beneficial to:
- Add try-catch blocks around KV operations
- Add type definitions for the entry values
- Consider masking sensitive data in the debug output
Here's a suggested implementation:
import { config } from "../lib/config.ts"; import { kv } from "../lib/db.ts"; + +// Define types for your KV entries +interface BadgeEntry { + name: string; + color: string; + // add other badge properties +} -console.log(`kvUrl - ${config.kvUrl} | env - ${Deno.env.get("DENO_KV_URL")}`); -const kvApi = await kv(config.kvUrl); +try { + console.log(`kvUrl - ${config.kvUrl} | env - ${Deno.env.get("DENO_KV_URL")}`); + const kvApi = await kv(config.kvUrl); -const badges = await kvApi.list({ prefix: ["staticBadges"] }); -const logos = await kvApi.list({ prefix: ["badgeIcons"] }); + const badges = await kvApi.list<BadgeEntry>({ prefix: ["staticBadges"] }); + const logos = await kvApi.list({ prefix: ["badgeIcons"] }); -for await (const entry of badges) { - console.log( - `badge namespace: ${entry.key[1]}, badge name: ${entry.key[2]}, data: ${ - JSON.stringify(entry.value) - }, stamp: ${entry.versionstamp}`, - ); -} + for await (const entry of badges) { + // Mask sensitive data if needed + const sanitizedValue = { ...entry.value, sensitiveField: '[REDACTED]' }; + console.log( + `badge namespace: ${entry.key[1]}, badge name: ${entry.key[2]}, data: ${ + JSON.stringify(sanitizedValue) + }, stamp: ${entry.versionstamp}`, + ); + } -for await (const entry of logos) { - console.log(`logo: ${entry.key[1]}, stamp: ${entry.versionstamp}`); + for await (const entry of logos) { + console.log(`logo: ${entry.key[1]}, stamp: ${entry.versionstamp}`); + } +} catch (error) { + console.error('Error while debugging KV store:', error); + Deno.exit(1); }api/meta.ts (1)
15-15: Consider simplifying the default value specification.The default value is specified twice: once in
Bool({ default: true })and again in.default(true). While this works, it's redundant.You could simplify it to either:
- ok: Bool({ default: true }).default(true), + ok: Bool({ default: true }),or
- ok: Bool({ default: true }).default(true), + ok: Bool().default(true),lib/config.ts (2)
1-4: Enhance documentation with detailed JSDoc comments.The current documentation could be more comprehensive. Consider adding:
- Description of each configuration option
- Type information
- Environment variable references
/** * @module - * @description Configuration + * @description Application configuration loaded from environment variables + * + * @property {number} port - Server port (env: PORT, default: 8080) + * @property {string} homepage - API homepage URL (env: BADGES_API_HOMEPAGE) + * @property {string} kvUrl - Deno KV URL (env: DENO_KV_URL) + * @property {Object} github - GitHub-related configuration + * @property {string} github.authServiceToken - GitHub API token (env: GITHUB_TOKEN) + * @property {string} github.org - GitHub organization (env: GITHUB_ORG) + * @property {string} github.team_slug - Admin team slug (env: GITHUB_TEAM_ADMINS) + * @property {string} cacheNamespace - Cache namespace (env: DENO_CACHE_NAMESPACE) + * @property {Object} flags - Feature flags + * @property {string} flags.edgeCache - Edge caching flag (env: FF_DENO_EDGE_CACHE) */
4-20: Consider using a configuration management library.The current configuration setup could benefit from:
- TypeScript interfaces for type safety
- Runtime validation using a schema validator (e.g., Zod)
- Environment-specific configuration handling
Example implementation:
import { z } from "https://deno.land/x/zod/mod.ts"; const ConfigSchema = z.object({ port: z.number().default(8080), homepage: z.string().url(), kvUrl: z.string().optional(), github: z.object({ authServiceToken: z.string().min(1), org: z.string().default("lorebooks-wiki"), team_slug: z.string().default("api-admins"), }), cacheNamespace: z.string(), flags: z.object({ edgeCache: z.boolean().default(false), }), }); type Config = z.infer<typeof ConfigSchema>; export const config = ConfigSchema.parse({ // ... your current config });.env (1)
10-10: Document the required GitHub token permissions.The secure implementation using encrypted values is good. However, please document the required GitHub token permissions and scope in a comment above the
GITHUB_TOKENvariable. This helps other developers create tokens with the minimum necessary permissions.Add a comment like this above the token:
+# GitHub Personal Access Token (Classic) +# Required permissions: read:org, repo (for private repos) GITHUB_TOKEN="encrypted:...".env.prod (1)
9-9: Consider documenting the GITHUB_TOKEN's purpose and required permissions.While the token is properly encrypted, it would be helpful to add a comment describing:
- The intended use of this token
- Required GitHub permissions/scopes
- Any rate limiting considerations
+# GitHub Personal Access Token for admin API authentication +# Required permissions: TBD GITHUB_TOKEN="encrypted:BIUCNVVVwehMW3+hmvN+SszVo+8pQB3wu/Gg1csjgcbf1XlEhe5ZTFdsUtzcZQFuJC5hd9wmGRrg71xPMP5ntrpUVmItps9mB+OdMEBmQPtexZjHBE1z6Ti2+oBYE4ghVszJV6TSnjRCydAEs0sNKM3FtPsPHd0xQlqt0xgV3WUdDZVpqPkn6AsJETjUrvEpvUcsoe+aF+KnpFjR39EWmJVUHJYiwmjlZXAmuAh5Z/0xql5BAr1Dv6d8S6HYXA=="api/admin.ts (1)
8-8: Rename class to follow naming conventions.The class name
testGitHubAuthsuggests this might be a test implementation. If this is a production endpoint, consider renaming it to something likeGitHubAuthRouteorCheckGitHubAuthRoute.lib/metadata.ts (1)
17-20: Add period for consistency in meta tag description.Other tag descriptions end with periods, maintain consistency.
Apply this diff:
{ name: "meta", - description: "Service meta endpoints" + description: "Service meta endpoints." }docs/api-terms.md (4)
Line range hint
1-4: Fix typo in the introduction.There's a typo in the introduction: "tp" should be "to".
-By using the Badges API, you agree tp and understand the following: +By using the Badges API, you agree to and understand the following:🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: There is an agreement error between ‘agree’ and ‘tp’. Insert ‘a(n)’ or change the noun to plural.
Context: ...ard Time) By using the Badges API, you agree tp and understand the following: - You fo...(PRP_VB_NN)
[uncategorized] ~10-~10: Possible missing comma found.
Context: ...point(s) for more than 5 requests per minute and respect the cache headers. - The AP...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~16-~16: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ablize things at any point in the future and you also agree to any revisions we may ...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~23-~23: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...ooks.wiki/legal/tos ## Privacy Policy Currently we do not collect IP addresses from eac...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
9-10: Enhance rate limit clause clarity.Consider adding specificity about the consequences of violating the rate limit and how the limit is enforced.
-for more than 5 requests per minute and respect the cache headers. +for more than 5 requests per minute and respect the cache headers. Exceeding this limit may result in temporary IP-based restrictions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: Possible missing comma found.
Context: ...point(s) for more than 5 requests per minute and respect the cache headers. - The AP...(AI_HYDRA_LEO_MISSING_COMMA)
36-37: Clarify issue reporting location.The term "here" is ambiguous. Consider providing a direct link to the GitHub repository's issues page.
-squad@crew.recaptime.dev` or file a new issue here. +squad@crew.recaptime.dev` or file a new issue in our [GitHub repository](https://github.com/lorebooks-wiki/badges-api/issues).
10-10: Improve readability with proper comma usage.Consider adding commas in the following locations for better readability:
- After "point(s)" in the rate limit clause
- Before "and" in the future changes clause
- After "Currently" in the privacy policy
-point(s) for more than 5 requests +point(s), for more than 5 requests -future and you also agree +future, and you also agree -Currently we do not collect +Currently, we do not collectAlso applies to: 16-16, 24-24
🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: Possible missing comma found.
Context: ...point(s) for more than 5 requests per minute and respect the cache headers. - The AP...(AI_HYDRA_LEO_MISSING_COMMA)
utils/preseed-kv.ts (1)
1-43: Consider adding transaction-like behavior and progress tracking.The current implementation could benefit from:
- Progress tracking to resume from failures
- Atomic operations to ensure data consistency
- Cleanup mechanism for partial failures
- Batch processing for better performance
This would make the script more reliable for large datasets and provide better visibility into the seeding process.
Would you like me to provide an example implementation with these improvements?
lib/db.ts (2)
67-67: LGTM with a suggestion for type safetyThe trailing comma addition maintains consistency. While the current implementation is solid, consider enhancing type safety by using a const assertion for the badge types.
Consider defining the badge types as a const:
export const BADGE_TYPES = ['redirect', 'badge'] as const; export type BadgeType = typeof BADGE_TYPES[number];Then update the function signature:
export async function setBadgeData( project: string, badgeName: string, type: BadgeType, data: BadgeData, )This would provide better type safety and autocompletion support.
Line range hint
3-11: Consider implementing connection pooling and stronger error typingThe current implementation creates a new KV connection for each operation. While this works, it could be optimized for performance at scale.
Consider these improvements:
- Implement connection pooling to reuse KV connections
- Add specific error types instead of using generic
string | object- Validate the KV URL format before attempting connection
- Add connection timeout handling
Example error type improvement:
export type DbError = { code: 'CONNECTION_ERROR' | 'NOT_FOUND' | 'INVALID_DATA'; message: string; details?: unknown; };docs/logos.md (4)
5-6: Consider adding a tracking link for the simple-icons integration.To help users track progress, consider adding a link to the relevant GitHub issue or project task.
-Looking for those available on `simple-icons` npm package? We're working on -integrating them soon. +Looking for those available on `simple-icons` npm package? We're working on +integrating them soon (track progress in [#issue-number]).
20-21: Improve grammar and clarity in the instructions.The word "clipboard" needs proper article usage.
-2. Run `cat path/to/file | base64 --wrap=0` and copy the encoded string to - clipboard. +2. Run `cat path/to/file | base64 --wrap=0` and copy the encoded string to the + clipboard.🧰 Tools
🪛 LanguageTool
[grammar] ~20-~20: The word ‘clipboard’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...4 --wrap=0and copy the encoded string to clipboard. 3. Open the [lib/cli/kv-seed.json`](....(VB_TO_NN_DT)
19-31: Consider adding example commands.To make the instructions more user-friendly, consider adding an example command with a sample file path.
1. Make sure the image or SVG in question is downloaded locally. 2. Run `cat path/to/file | base64 --wrap=0` and copy the encoded string to clipboard. + Example: `cat assets/logos/example-logo.svg | base64 --wrap=0`🧰 Tools
🪛 LanguageTool
[grammar] ~20-~20: The word ‘clipboard’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...4 --wrap=0and copy the encoded string to clipboard. 3. Open the [lib/cli/kv-seed.json`](....(VB_TO_NN_DT)
40-41: Fix grammar in CLI instructions.The word "clipboard" needs proper article usage.
-2. Run `cat path/to/file | base64 --wrap=0` and copy the encoded string to - clipboard. +2. Run `cat path/to/file | base64 --wrap=0` and copy the encoded string to the + clipboard.🧰 Tools
🪛 LanguageTool
[grammar] ~40-~40: The word ‘clipboard’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...4 --wrap=0and copy the encoded string to clipboard. 3. Rundotenvx run -f .env.prod --...(VB_TO_NN_DT)
lib/cli/logger.ts (1)
Line range hint
1-54: Consider architectural improvements for better type safety.The codebase could benefit from several TypeScript improvements:
- Replace the levels object with a proper TypeScript enum
- Add proper type definitions instead of using @ts-expect-error
- Implement proper type safety for the setLogLevel function
Here's an example of how to improve the types:
enum LogLevel { ERROR = 0, WARN = 1, INFO = 2, VERBOSE = 4, DEBUG = 5, SILLY = 6 } interface LogOptions { debug?: boolean; verbose?: boolean; quiet?: boolean; logLevel?: keyof typeof LogLevel; } // Then use these types in your functions export function setLogLevel(options: LogOptions): void { // ... implementation }Would you like me to provide a complete type-safe implementation for the logging system?
docs/hackclub-badges.md (3)
3-5: Minor grammar improvements needed.Consider these refinements:
- Add "the" before "Hack Club Slack community"
- Remove the hyphen in "fiscally hosted" as it's redundant with the -ly ending
-These badges are maintained by @ajhalili2006 for the benefit of Hack Club Slack +These badges are maintained by @ajhalili2006 for the benefit of the Hack Club Slack community, especially those who participated during Arcade 2024 and open-source -projects fiscally-hosted under HCB. +projects fiscally hosted under HCB.🧰 Tools
🪛 LanguageTool
[style] ~3-~3: ‘for the benefit’ might be wordy. Consider a shorter alternative.
Context: ... badges are maintained by @ajhalili2006 for the benefit of Hack Club Slack community, especiall...(EN_WORDINESS_PREMIUM_FOR_THE_BENEFIT)
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ...ned by @ajhalili2006 for the benefit of Hack Club Slack community, especially those ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~5-~5: The hyphen in fiscally-hosted is redundant.
Context: ...ng Arcade 2024 and open-source projects fiscally-hosted under HCB. ## Hack Clubber? If you're...(ADVERB_LY_HYPHEN_FIX)
61-63: Minor grammar improvements needed in the introduction.Consider these refinements:
- Add "the" before "hackclub-arcade tag"
- Add a comma after "tag"
Built a project during [Arcade 2024](https://hackclub.com/arcade), including [Power Hour: Arcade](https://hackclub.com/arcade/power-hour)? Use this badge, -alongside adding `hackclub-arcade` tag to your repository for discoverability. +alongside adding the `hackclub-arcade` tag, to your repository for discoverability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~63-~63: You might be missing the article “the” here.
Context: ...hour)? Use this badge, alongside addinghackclub-arcadetag to your repository ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~63-~63: Possible missing comma found.
Context: ...dge, alongside addinghackclub-arcadetag to your repository for discoverability....(AI_HYDRA_LEO_MISSING_COMMA)
126-128: Maintain consistent hyphenation throughout the document.Remove the redundant hyphens in "fiscally-hosted" across these sections to match the earlier correction.
-For organizations fiscally-hosted through HCB, you can use the Donate on HCB to +For organizations fiscally hosted through HCB, you can use the Donate on HCB to add a donation link for your org or event. Just override the org slug by adding `org` query parameter pointing to your org. -For organizations fiscally-hosted through HCB, you can also show your balances +For organizations fiscally hosted through HCB, you can also show your balances as a badge while linking to your organization's public ledger on the platform. Just override the org slug by adding `org` query parameter pointing to your org.Also applies to: 135-137
🧰 Tools
🪛 LanguageTool
[uncategorized] ~126-~126: The hyphen in fiscally-hosted is redundant.
Context: ...`` ## Donate on HCB For organizations fiscally-hosted through HCB, you can use the Donate on ...(ADVERB_LY_HYPHEN_FIX)
utils/manage-api-users.ts (1)
Line range hint
1-173: Add rate limit handling and input validationConsider implementing these security improvements:
- Add rate limit handling with exponential backoff
- Validate username input parameters
- Add confirmation prompts for destructive operations
Would you like me to provide an implementation for these security improvements?
🧰 Tools
🪛 Biome
[error] 130-130: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
utils/badges-admin.ts (2)
Line range hint
89-152: Add input validation and consistent error handling in badges:add command.The command needs more robust input validation and consistent error handling:
- Missing validation for project and name parameters
- No URL validation for redirect type
- Inconsistent error handling patterns
.action(async (project, name, options) => { + // Validate project and name + if (!/^[a-z0-9-]+$/.test(project) || !/^[a-z0-9-]+$/.test(name)) { + logger.error('Project and name must contain only lowercase letters, numbers, and hyphens'); + Deno.exit(1); + } + logger.debug(`options ${JSON.stringify(options)}`); if (options.type == "redirect") { - if (options.url == undefined) { - logger.error(`you forgot to set --url option`); + if (!options.url) { + logger.error('Missing required --url option for redirect type'); + Deno.exit(1); + } + + try { + new URL(options.url); // Validate URL format + } catch { + logger.error('Invalid URL format'); + Deno.exit(1); }
Based on the analysis of the codebase, I can now generate the final response:
Security measures are partially implemented but need enhancement
The codebase has some security measures in place:
- Style validation through
validateBadgeStylefunction that restricts to known safe values- SVG content type validation for uploaded icons
- Badge generation through the trusted
badge-makerpackageHowever, several security concerns remain:
- No input sanitization for badge text content (label, message)
- No validation of redirect URLs
- No rate limiting for KV operations
- No validation of base64-encoded logo data beyond basic prefix checks
🔗 Analysis chain
Line range hint
1-193: Security: Add input sanitization and rate limiting.Consider implementing these security measures:
- Sanitize all user inputs (especially badge parameters) to prevent injection attacks
- Implement rate limiting for KV operations
- Validate SVG content before storage to prevent XSS
Let's check for any existing security measures in the codebase:
Let's check for any existing security measures in the codebase, particularly around input validation, rate limiting, and SVG security. The previous search was too broad, so I've narrowed it down to specific patterns and file types.
The previous search found files handling SVG content. Let's examine their implementation to understand the security measures in place.
From the previous output, I can see that badge generation is handled by the
badge-makerpackage and there's avalidateBadgeStylefunction being used. Let's check these validation mechanisms and the database layer to complete our security analysis.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for input validation and rate limiting rg -i "sanitize|rate.?limit|xss|injection" # Check for security-related dependencies rg -g "package*" "security|sanitize|rate.?limit"Length of output: 36666
Script:
#!/bin/bash # Check for input validation and sanitization ast-grep --pattern 'validate($$$)' ast-grep --pattern 'sanitize($$$)' # Check for rate limiting middleware or similar rg -l "rate.?limit|throttle" --type ts # Check for SVG validation rg -l "svg|xss" --type ts # Check badge-maker package security rg -g "package*" "badge-maker"Length of output: 358
Script:
#!/bin/bash # Let's examine the SVG handling in the identified files rg -A 10 -B 10 "svg|xss" utils/badges-admin.ts api/badges.ts # Check for any input validation in badge generation rg -A 10 -B 10 "makeBadge" utils/badges-admin.ts api/badges.ts # Look for any security-related checks in badge data handling rg -A 10 -B 10 "badgeData|redirectUrl" utils/badges-admin.ts api/badges.tsLength of output: 24597
Script:
#!/bin/bash # Check for input validation in utils rg -A 5 "validate" lib/utils.ts # Check badge-maker package version and any security features rg -g "package*|deno.json*" "badge-maker" # Check if there's any sanitization in the db layer rg -A 10 "setBadgeData" lib/db.tsLength of output: 939
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.env(1 hunks).env.prod(1 hunks).gitignore(1 hunks).tool-versions(1 hunks).vscode/extensions.json(1 hunks).vscode/settings.json(1 hunks)api/admin.ts(1 hunks)api/badges.ts(12 hunks)api/meta.ts(1 hunks)deno.json(1 hunks)docs/README.md(1 hunks)docs/api-terms.md(1 hunks)docs/hackclub-badges.md(2 hunks)docs/logos.md(1 hunks)docs/self-hosting.md(1 hunks)lib/cli/kv-seed.json(3 hunks)lib/cli/logger.ts(2 hunks)lib/config.ts(1 hunks)lib/db.ts(2 hunks)lib/githubAuth.ts(1 hunks)lib/logos.ts(1 hunks)lib/metadata.ts(2 hunks)lib/utils.ts(1 hunks)main.ts(2 hunks)utils/badges-admin.ts(7 hunks)utils/kv-keys-debugger.ts(1 hunks)utils/manage-api-users.ts(2 hunks)utils/preseed-kv.ts(1 hunks)utils/types.ts(2 hunks)
🧰 Additional context used
🪛 Biome
api/badges.ts
[error] 53-53: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
utils/manage-api-users.ts
[error] 130-130: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🪛 LanguageTool
docs/api-terms.md
[uncategorized] ~10-~10: Possible missing comma found.
Context: ...point(s) for more than 5 requests per minute and respect the cache headers. - The AP...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~16-~16: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ablize things at any point in the future and you also agree to any revisions we may ...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~23-~23: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...ooks.wiki/legal/tos ## Privacy Policy Currently we do not collect IP addresses from eac...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
docs/hackclub-badges.md
[style] ~3-~3: ‘for the benefit’ might be wordy. Consider a shorter alternative.
Context: ... badges are maintained by @ajhalili2006 for the benefit of Hack Club Slack community, especiall...
(EN_WORDINESS_PREMIUM_FOR_THE_BENEFIT)
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ...ned by @ajhalili2006 for the benefit of Hack Club Slack community, especially those ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~5-~5: The hyphen in fiscally-hosted is redundant.
Context: ...ng Arcade 2024 and open-source projects fiscally-hosted under HCB. ## Hack Clubber? If you're...
(ADVERB_LY_HYPHEN_FIX)
[uncategorized] ~63-~63: You might be missing the article “the” here.
Context: ...hour)? Use this badge, alongside adding hackclub-arcade tag to your repository ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~63-~63: Possible missing comma found.
Context: ...dge, alongside adding hackclub-arcade tag to your repository for discoverability....
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~68-~68: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 2470 characters long)
Context: ...hackclub.com/arcade) Other styles:
[
[uncategorized] ~126-~126: The hyphen in fiscally-hosted is redundant.
Context: ...`` ## Donate on HCB For organizations fiscally-hosted through HCB, you can use the Donate on ...
(ADVERB_LY_HYPHEN_FIX)
[uncategorized] ~135-~135: The hyphen in fiscally-hosted is redundant.
Context: ...hq) ## HCB Balances For organizations fiscally-hosted through HCB, you can also show your bal...
(ADVERB_LY_HYPHEN_FIX)
docs/logos.md
[grammar] ~20-~20: The word ‘clipboard’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...4 --wrap=0 and copy the encoded string to clipboard. 3. Open the [lib/cli/kv-seed.json`](....
(VB_TO_NN_DT)
[grammar] ~40-~40: The word ‘clipboard’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...4 --wrap=0and copy the encoded string to clipboard. 3. Run dotenvx run -f .env.prod --...
(VB_TO_NN_DT)
docs/self-hosting.md
[uncategorized] ~14-~14: “you” seems less likely than “you’re” (you are).
Context: ...r PAT (via DENO_KV_ACCESS_TOKEN) if you doing presistence on the cloud. - `DENO...
(AI_HYDRA_LEO_CP_YOU_YOUARE)
[uncategorized] ~26-~26: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...nd. 3. Pre-seed the KV backend from our manually-crafted production export: `deno task utils:...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[uncategorized] ~28-~28: You might be missing the article “an” here.
Context: ...admin APIs with your GitHub PAT, create api-admins team on your organization...
(AI_EN_LECTOR_MISSING_DETERMINER_AN)
[uncategorized] ~28-~28: The preposition “in” seems more likely in this position than the preposition “on”.
Context: ...ur GitHub PAT, create api-admins team on your organization. (or reuse a exist...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_ON_IN)
[misspelling] ~29-~29: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...team on your organization. (or reuse a existing team) and use the team slug as...
(EN_A_VS_AN)
🔇 Additional comments (27)
.gitignore (1)
4-4: LGTM! Verify pattern effectiveness.
The wildcard pattern *.decrypted is a good security practice to prevent any decrypted sensitive files from being accidentally committed.
Let's verify that the pattern works as expected:
✅ Verification successful
Pattern *.decrypted works correctly for ignoring sensitive files
The verification confirms:
- No
.decryptedfiles are currently tracked in the repository - The pattern successfully ignores files with
.decryptedextension in both root and subdirectories - Git correctly recognizes and applies the pattern from line 4 of
.gitignore
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the .gitignore pattern correctly ignores .decrypted files
# and that no such files are currently tracked.
# Test 1: Check if any .decrypted files are currently tracked
echo "Checking for tracked .decrypted files..."
git ls-files "*.decrypted"
# Test 2: Verify the pattern is recognized by git
echo "Verifying .gitignore pattern..."
git check-ignore -v "test.decrypted"
git check-ignore -v "subfolder/secret.decrypted"
Length of output: 370
.vscode/extensions.json (1)
2-5: LGTM! The recommended extensions are appropriate for the project.
The extensions recommended are essential for the development workflow:
denoland.vscode-deno: Provides Deno language support and toolingvivaxy.vscode-conventional-commits: Helps maintain consistent commit messages
Let's verify the project actually uses Deno:
✅ Verification successful
✅ The Deno extension recommendation is appropriate for this project
This is a Deno project as evidenced by:
- Presence of
deno.jsonwith comprehensive Deno configuration - Deno-specific tasks for running, deploying, and managing the application
- TypeScript files using
.tsextension with Deno-style imports - Project uses Deno Deploy for deployment
The conventional commits extension is also appropriate for maintaining consistent commit messages in any project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Deno usage in the project
# Test: Look for Deno configuration files and imports
echo "Checking for Deno configuration..."
rg -l "deno" && fd -g "deno.{json,jsonc}"
Length of output: 208
Script:
#!/bin/bash
# Let's verify the actual content of deno.json and check imports
echo "=== Checking deno.json configuration ==="
cat deno.json
echo -e "\n=== Checking Deno imports in TypeScript files ==="
rg "^import.*from.*" lib/metadata.ts
Length of output: 2046
docs/README.md (2)
10-10: LGTM!
The license declaration is clear and appropriate.
3-6: Verify documentation links accessibility.
The links appear to be well-structured, but let's verify they are accessible and up-to-date.
✅ Verification successful
Documentation links are valid and well-maintained
All local documentation files exist and contain appropriate content:
api-terms.mdcontains API terms with recent updateshackclub-badges.mddescribes the Hack Club badges functionalitylogos.mdcovers badge logo information- Admin API documentation is present in
self-hosting.md
The external API docs link (badges.api.lorebooks.wiki/docs) is the only link that can't be verified in the sandbox environment, but it follows a standard subdomain pattern for API documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all referenced documentation files exist in the repository
echo "Checking local documentation files..."
for file in "api-terms.md" "hackclub-badges.md" "logos.md"; do
if [ -f "docs/$file" ]; then
echo "✓ docs/$file exists"
else
echo "⨯ docs/$file is missing"
fi
done
Length of output: 555
Script:
#!/bin/bash
# Let's verify the content of these files to ensure they are properly formatted markdown files
# and contain relevant content
echo "=== Content of api-terms.md ==="
head -n 5 docs/api-terms.md
echo -e "\n=== Content of hackclub-badges.md ==="
head -n 5 docs/hackclub-badges.md
echo -e "\n=== Content of logos.md ==="
head -n 5 docs/logos.md
# Also check if there's any admin API documentation
echo -e "\nSearching for admin API documentation..."
rg -i "admin.*api|api.*admin" docs/ -A 2
Length of output: 1802
.vscode/settings.json (2)
13-14: LGTM: Conventional commits scope additions are appropriate.
The addition of "global" and "vscode" scopes to conventional commits configuration makes sense as they represent common areas of change in the codebase.
22-22: Verify impact of disabling dotenv autocloaking.
Disabling dotenv.enableAutocloaking means environment variables will be visible in the editor. While this can be helpful during development, ensure this doesn't lead to accidental commits of sensitive information.
Let's check if there are any unprotected environment files:
✅ Verification successful
Let me check the content of these .env files and .gitignore to better understand the situation.
Environment files are properly secured with encryption
The environment files (.env and .env.prod) are tracked in the repository but they contain only encrypted values using dotenv's public-key encryption system. All sensitive values are encrypted with the prefix "encrypted:" and the files include the corresponding public keys. This is a secure practice as the private keys needed for decryption are not stored in the repository.
.gitignorecorrectly excludes.env.keyswhich likely contains the private keys- Both
.envand.env.produse proper encryption for all sensitive values - The autocloaking setting is safe in this context as it only affects how VSCode displays already-encrypted values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for .env files that aren't properly gitignored
# Expected: All .env files should be in .gitignore
# Test 1: Check if .env files are properly gitignored
rg -l "\.env" .gitignore
# Test 2: List any tracked .env files that might expose secrets
git ls-files | rg "\.env"
Length of output: 80
Script:
#!/bin/bash
# Check the content of .gitignore for .env patterns
echo "=== .gitignore patterns ==="
rg "\.env" .gitignore
# Check if .env files contain actual secrets or are just templates
echo -e "\n=== .env file content ==="
cat .env || echo "Cannot read .env"
echo -e "\n=== .env.prod file content ==="
cat .env.prod || echo "Cannot read .env.prod"
Length of output: 2467
lib/utils.ts (1)
1-16: Verify the impact of style validation.
The function is used in badge generation, so let's verify its integration.
✅ Verification successful
Let me analyze the usage of validateBadgeStyle further by checking the implementation in api/badges.ts to understand the complete flow.
Style validation implementation is correctly integrated and safe
The validation function is properly integrated across the codebase:
- Used consistently in all badge generation endpoints (
hcbBalanceOps,hcbDonateButton,generateSvg) - Matches the type definition in
lib/db.tswhich defines the valid style values - Provides a safe fallback to "flat" style when validation fails
- Correctly handles both direct style parameter and style from stored badge data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of validateBadgeStyle and potential edge cases
# Test 1: Find all imports of validateBadgeStyle
echo "=== Checking imports ==="
rg "import.*validateBadgeStyle" -A 2
# Test 2: Find all direct usage of the function
echo -e "\n=== Checking function usage ==="
rg "validateBadgeStyle\(" -A 2
# Test 3: Check if there are any hardcoded badge styles that might need validation
echo -e "\n=== Checking for hardcoded styles ==="
rg -i "style: ['\"](?:plastic|flat|flat-square|for-the-badge|social)['\"]" -A 2
Length of output: 1468
Script:
#!/bin/bash
# Check the implementation in api/badges.ts to understand how the style parameter is handled
echo "=== Checking badges.ts implementation ==="
cat api/badges.ts
# Also check if there are any tests for validateBadgeStyle
echo -e "\n=== Checking for tests ==="
fd "test" --exec grep -l "validateBadgeStyle"
Length of output: 10734
utils/kv-keys-debugger.ts (1)
12-14: LGTM! The console log formatting looks cleaner.
The template string formatting change improves readability while maintaining the same output information.
api/meta.ts (2)
6-8: LGTM! Good use of TypeScript features and OpenAPI specifications.
The override keyword and tags implementation follow best practices. The endpoint is properly categorized under "meta" which improves API documentation organization.
Line range hint 23-28: LGTM! Clean and consistent implementation.
The handle method is properly overridden and its implementation matches the defined schema. The response structure is clear and appropriate for a health check endpoint.
.env.prod (1)
8-9: LGTM! Proper security measures in place.
The environment variables are properly encrypted using dotenvx.com's encryption mechanism, which is a secure approach for storing sensitive credentials in version control.
api/admin.ts (1)
20-40: Consider adding rate limiting for the authentication endpoint.
To prevent potential brute-force attacks or API abuse, consider implementing rate limiting for this authentication endpoint.
Let's check if rate limiting is implemented elsewhere:
lib/metadata.ts (2)
39-39: LGTM!
The updated description provides better clarity about the development environment.
43-43: LGTM!
The added newline improves readability.
deno.json (1)
8-8: LGTM! Good practice to specify version constraints.
The addition of version constraint ^21.0.2 for @octokit/rest is a good practice as it:
- Locks the major version to prevent breaking changes
- Allows minor and patch updates for bug fixes and compatible features
Let's verify version compatibility with other dependencies:
✅ Verification successful
Version constraint for @octokit/rest is appropriate and safely used
The codebase uses @octokit/rest in a standard way for GitHub API authentication and team management. The version constraint ^21.0.2 is appropriate as:
- It's used for basic Octokit instantiation with auth tokens
- No deprecated or version-specific APIs are being used
- The implementation follows current Octokit patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any version conflicts or related dependencies
# Look for other @octokit related imports or version specifications
rg -i "octokit" --type json
Length of output: 87
Script:
#!/bin/bash
# Let's check for actual usage of @octokit/rest in the codebase
rg -l "@octokit/rest"
# Also check for any TypeScript/JavaScript files that might use Octokit
rg -l "Octokit"
Length of output: 145
Script:
#!/bin/bash
# Let's examine how @octokit/rest is used in these files to ensure compatibility
rg -A 5 -B 5 "@octokit/rest" lib/githubAuth.ts utils/manage-api-users.ts
Length of output: 990
lib/db.ts (1)
38-38: LGTM: Style consistency improvement
The addition of a trailing comma aligns with modern TypeScript style conventions and makes future parameter additions cleaner in version control.
docs/logos.md (1)
46-48: LGTM!
The footnote provides clear and helpful information about the CDN URLs.
docs/hackclub-badges.md (2)
11-17: LGTM! Clear instructions for Hack Club HQ members.
The section provides clear guidance with proper links to utilities and communication channels.
21-57: LGTM! Comprehensive badge usage documentation.
The section provides clear examples with:
- Visual demonstrations of all badge styles
- Well-formatted markdown code blocks
- Proper linking to the Hack Club website
lib/cli/kv-seed.json (3)
3-7: LGTM: Icon updates improve theme support
The changes appropriately split the HCB icon into light and dark variants while adding a new arcade-specific icon. This improves theme support and visual consistency.
33-33: LGTM: Badge logo update matches new icon naming
The badge logo update from "hackclub" to "hackclub-light" maintains consistency with the new light/dark icon variants.
46-49: LGTM: Badge configuration updates improve UX
The changes enhance the HCB badge by:
- Using the new dark variant of the HCB icon
- Updating links to more specific and relevant URLs
main.ts (4)
1-11: LGTM!
The new imports are relevant and necessary for the added functionality in this file.
14-26: Verify the CORS configuration.
The CORS configuration looks good. It allows all origins, exposes the necessary headers, and supports the required HTTP methods. However, please ensure that the credentials option is intentionally set to true. Setting it to true means that the browser will include cookies in cross-origin requests, which may have security implications if not properly handled.
80-99: Ensure proper error handling and logging for the GitHub authentication process.
The implementation of the GitHub authentication using the bearerAuth middleware and the handleGitHubAuth function looks good. However, it's important to ensure proper error handling and logging throughout the authentication process. Consider the following:
-
In the
verifyTokenfunction, add error handling for the case whenhandleGitHubAuththrows an error or returnsfalse. You may want to log the error and return an appropriate error response to the client. -
Inside the
handleGitHubAuthfunction (assuming it's defined inlib/githubAuth.ts), ensure that any errors that occur during the authentication process are properly caught, logged, and handled. This includes errors that may occur when making API requests to GitHub or processing the response. -
Consider adding logging statements at key points in the authentication flow to help with debugging and monitoring. For example, you can log when a token is successfully verified or when an authentication error occurs.
If you need assistance with implementing the error handling and logging, I can provide more detailed suggestions or code examples. Just let me know!
105-109: LGTM!
The new route for /hcb/balances correctly redirects to the /hcb/balance endpoint while preserving any query parameters. This is a good way to handle alternative route names and maintain compatibility.
api/badges.ts (1)
207-210:
Define color and links in the request query schema
You're destructuring color and links from apiReqData.query, but these parameters are not defined in your query schema. As a result, they will always be undefined.
Add color and links to your request query schema to ensure they are properly validated and available:
request: {
query: z.object({
json: Bool({
required: false,
description: "Whether to force pull JSON data from KV or not",
}),
style: Str({
description: "Badge style as supported by `badge-maker` npm library.",
required: false,
default: "flat",
}),
+ color: Str({
+ description: "Color of the badge.",
+ required: false,
+ }).optional(),
+ links: z.array(Str()).optional(),
}),
},Likely invalid or redundant comment.
…rabbitai Including some QoL improvements to parts of the codebase. Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Also finalize self-hosting docs ftw Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
…for Deno.KV.get Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
…ions Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <ajhalili2006@crew.recaptime.dev>
|
I might ship this with just the barebones for now and implement the rest of the API later. |
|
I probably forgot this lol, but will work on it being merged soon :) |
There was a problem hiding this comment.
Pull request overview
This PR introduces a GitHub PAT–protected Admin API to the Deno/Hono backend, adds edge caching for selected routes, and expands/updates project documentation and scripts to support self-hosting and administration.
Changes:
- Adds
/admin/*endpoints with bearer-token authentication backed by GitHub org/team membership, plus KV-based caching of auth checks. - Adds Cache API edge-caching for
/hcb/*and/badges/*routes and extends HCB badge functionality (e.g., total raised). - Updates docs, tooling, and repo metadata (Deno lock/imports, scripts, contributor docs, VS Code settings).
Reviewed changes
Copilot reviewed 31 out of 34 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/types.ts | Adds a DB-store type wrapper for KV badge payloads. |
| utils/preseed-kv.ts | Improves KV seeding flow and error handling; replaces async forEach with for..of. |
| utils/manage-api-users.ts | Refactors admin-user management CLI and improves error messaging. |
| utils/kv-keys-debugger.ts | Minor formatting/log output adjustment. |
| utils/badges-admin.ts | Tightens icon upload validation and modifies badge/redirect creation logic. |
| main.ts | Adds edge cache middleware and introduces /admin/* auth + write-auth for /badges/*. |
| lib/utils.ts | Adds badge-style validation helper. |
| lib/metadata.ts | Updates OpenAPI tags/metadata and description text. |
| lib/logos.ts | Replaces prior logo logic with simple-icons resolution + KV caching. |
| lib/hcb.ts | Updates module docstrings. |
| lib/githubAuth.ts | Adds GitHub PAT auth handling with KV caching and token hashing. |
| lib/db.ts | Adds typed KV reads and updates icon resolution behavior. |
| lib/config.ts | Adds config/docs and GitHub token validation helper; adds cache-related flags. |
| lib/cli/logger.ts | Formatting cleanup and object-message handling tweak. |
| docs/self-hosting.md | Expands env var docs and deployment instructions. |
| docs/logos.md | Updates logo docs and KV seeding instructions. |
| docs/hackclub-badges.md | Adds usage examples and expands HCB badge documentation. |
| docs/deno-scripts.md | New doc page describing common Deno scripts/tasks. |
| docs/api-terms.md | Updates ToS text, privacy notes, and adds Hack Club badges addendum. |
| docs/README.md | Restructures docs index and adds contributor/self-hosting pointers. |
| deno.lock | Updates lockfile format and dependency versions. |
| deno.json | Pins Octokit version; minor deploy config tweak. |
| api/meta.ts | Adds OpenAPI tags and uses override for schema/handle. |
| api/badges.ts | Adds HCB “total raised”, caching headers, style validation, and adjusts JSON/SVG response logic. |
| api/admin.ts | New Admin API routes for auth testing and granting admin access. |
| README.md | Updates project branding, adds badges, and clarifies local run instructions. |
| CONTRIBUTING.md | Adds contributor onboarding and patch-submission guidelines. |
| .vscode/settings.json | Adds editor/conventional-commit related settings. |
| .vscode/extensions.json | Formats extension recommendations. |
| .tool-versions | Pins Deno version for tool managers. |
| .gitignore | Broadens decrypted-file ignore pattern. |
| .env.prod | Adds encrypted GITHUB_TOKEN for prod env. |
| .env | Adds encrypted GITHUB_TOKEN for dev env. |
Comments suppressed due to low confidence (2)
api/badges.ts:288
apiReqData.queryschema only definesjsonandstyle, but the handler destructurescolorandlinks. This will fail TypeScript type-checking and also indicates these query params are either missing from the schema or should be removed.
const reqUrl = new URL(c.req.url);
const { origin } = reqUrl;
const { color, links, style } = apiReqData.query;
utils/badges-admin.ts:127
- For
--type redirect,datais typed asBadgeDatabut onlyredirectUrlis provided. For--type badge, required fields likeredirectUrlare omitted. This will not type-check and reflects the underlyingBadgeDatatype issue—split redirect vs badge payload types (or make the type a discriminated union) and build the correct shape for each branch.
if (options.type == "redirect") {
if (options.url == undefined) {
logger.error(`you forgot to set --url option`);
}
const data: BadgeData = {
redirectUrl: options.url,
};
const result = await db.setBadgeData(project, name, "redirect", data);
if (result.ok == true) {
logger.success(
`added redirect on https://badges.api.lorebooks.wiki/badges/${project}/${name}`,
);
}
}
if (options.type == "badge") {
const data: BadgeData = {
label: options.label || null,
labelColor: options.labelColor,
color: options.color,
message: options.message,
logo: options.logo || null,
style: options.style,
links: options.links || null,
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export async function resolveBadgeIcon(icon: string) { | ||
| const kvApi = await kv(config.kvUrl); | ||
| let result: string | null | ||
| if (icon == null) { | ||
| return null; | ||
| return undefined; | ||
| } |
| @@ -1,19 +1,50 @@ | |||
| import { Hono } from "hono"; | |||
| import { Context, Hono, Next } from "hono"; | |||
| const msgBuffer = (msg: string) => new TextEncoder().encode(msg); | ||
| export const github = (token: string) => new Octokit({ | ||
| auth: token, | ||
| userAgent: `@lorebooks-wiki/badges-api $({config.homepage})` |
| const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8"); | ||
| const result = `data:image/svg+xml;base64,${svgContentEncoded}`; |
| message: data.message, | ||
| color: data?.color || "gray", | ||
| color: color || data?.color || "gray", | ||
| style: validateBadgeStyle(style || data?.color), |
| } | ||
|
|
||
| /** | ||
| * Hash a GitHub token into SHA-215 encoded text. |
| if (!iconName.startsWith("si:")) { | ||
| throw new Error("not a simple-icon icon name") | ||
| } | ||
| } | ||
|
|
||
| return svg2base64(iconSvg); | ||
| } | ||
|
|
||
| function prepareNamedLogo({ name, color, style, size }) { | ||
| if (typeof name !== "string") { | ||
| return undefined; | ||
| } | ||
| // check on our KV cache first | ||
| const cacheKv = await kvApi.get<string>(["badgeIcons", iconName]) | ||
|
|
||
| name = name.toLowerCase(); | ||
| // if not cached OR the cache busting is enabled, run the resolver | ||
| if (cacheKv.value === null || cacheBuster === true) { | ||
| const upstreamData = await resolveSimpleIcon(iconName) | ||
|
|
| const { org, style } = apiReqData.query; | ||
| const dbData = (await getBadgeData("hcb", "donate")).result?.data; | ||
| const badgeData: Format = { | ||
| label: dbData?.label, | ||
| labelColor: dbData?.labelColor, | ||
| logoBase64: await resolveBadgeIcon("hcb-dark"), | ||
| message: `Donate to ${org || "HQ"}`, | ||
| links: [ | ||
| `https://hcb.hackclub.com/donations/start/${org || "hq"}`, | ||
| `https://hcb.hackclub.com/donations/start/${org || "hq"}`, | ||
| ], | ||
| }; | ||
| const badge = makeBadge(badgeData); | ||
| return c.newResponse(badge, 200, { | ||
| "Content-Type": "image/svg+xml", | ||
| }); | ||
| const { result, code } = await getOrgData(org || "hq"); |
| kvUrl: Deno.env.get("DENO_KV_URL"), | ||
| github: { | ||
| authServiceToken: Deno.env.get("GITHUB_TOKEN"), | ||
| authServiceToken: getValidatedGithubToken(), |
| export class hcbTotalRasied extends OpenAPIRoute { | ||
| override schema = { |
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utils/badges-admin.ts (1)
101-115:⚠️ Potential issue | 🔴 CriticalAbort redirect flow on missing
--urland handle failed writes.Right now, missing
--urlonly logs an error and still proceeds. Also, redirect write failures are not surfaced (result.ok == falsepath is missing).🛠️ Suggested patch
if (options.type == "redirect") { if (options.url == undefined) { logger.error(`you forgot to set --url option`); + Deno.exit(1); } const data: BadgeData = { redirectUrl: options.url, }; const result = await db.setBadgeData(project, name, "redirect", data); - - if (result.ok == true) { - logger.success( - `added redirect on https://badges.api.lorebooks.wiki/badges/${project}/${name}`, - ); - } + if (!result.ok) { + logger.error("Something went wrong while adding a redirect badge."); + logger.warn(result.error); + Deno.exit(1); + } + logger.success( + `added redirect on https://badges.api.lorebooks.wiki/badges/${project}/${name}`, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/badges-admin.ts` around lines 101 - 115, When options.type === "redirect" abort early if options.url is undefined by logging the error and returning/throwing before building BadgeData or calling db.setBadgeData; ensure you reference options.url, options.type and the BadgeData shape. After calling db.setBadgeData(project, name, "redirect", data) check result.ok and on false log an error (including result.error or result message) and exit non-zero (or throw) instead of silently continuing; on true keep the existing logger.success path. This touches the redirect branch in utils/badges-admin.ts, specifically the code around db.setBadgeData, BadgeData, logger.error, and logger.success.
🟡 Minor comments (17)
lib/metadata.ts-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorCorrect public-facing metadata typos.
Two strings appear in API docs output:
communtiesandlocal host.✏️ Suggested patch
- "HCB badges for use by organizations and communties fiscally sponsored by Hack Club through the HCB platform.", + "HCB badges for use by organizations and communities fiscally sponsored by Hack Club through the HCB platform.", ... - description: "Development instance (local host)", + description: "Development instance (localhost)",Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/metadata.ts` at line 11, Fix the typos in the public-facing metadata strings in lib/metadata.ts: change "communties" to "communities" in the string "HCB badges for use by organizations and communties fiscally sponsored by Hack Club through the HCB platform." and change any "local host" occurrences to "localhost" (also present elsewhere around the same file, e.g., the second occurrence near line 39). Update these literal strings so the exported metadata values (the description entries) are corrected.docs/README.md-3-4 (1)
3-4:⚠️ Potential issue | 🟡 MinorFix typo in the intro sentence.
exisitngshould beexisting.✏️ Suggested patch
-Welcome to the Markdown documentation for our Badges API! We welcome improvements to this documentation if -you found any issues with it or even expand exisitng content. +Welcome to the Markdown documentation for our Badges API! We welcome improvements to this documentation if +you found any issues with it or even expanded existing content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/README.md` around lines 3 - 4, Fix the typo in the README intro sentence: change the word "exisitng" to "existing" in the first paragraph ("Welcome to the Markdown documentation for our Badges API! ... expand exisitng content.") so the sentence reads "... expand existing content.".README.md-32-36 (1)
32-36:⚠️ Potential issue | 🟡 MinorFix minor wording issues in the local/self-hosting section.
There are two text issues:
overriden→overridden, anda more detailed instructions→more detailed instructions.✏️ Suggested patch
-It will listen on `localhost:8080` by default unless overriden by `PORT` +It will listen on `localhost:8080` by default unless overridden by `PORT` environment variable. -[See the docs](./docs/self-hosting.md) for a more detailed instructions on self-hosting +[See the docs](./docs/self-hosting.md) for more detailed instructions on self-hosting and more.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 32 - 36, Update the README local/self-hosting text to fix two typos: change "overriden" to "overridden" in the sentence mentioning PORT and change "a more detailed instructions" to "more detailed instructions" in the "See the docs" line (the line referencing "localhost:8080", "PORT", and "./docs/self-hosting.md"). Ensure the corrected phrases appear exactly where those original tokens are used.docs/self-hosting.md-16-17 (1)
16-17:⚠️ Potential issue | 🟡 MinorFix typo in
DENO_KV_ACCESS_TOKENdescription.
authenicate→authenticate.✏️ Suggested patch
-- `DENO_KV_ACCESS_TOKEN` - Used to authenicate against a KV Connect-compatible +- `DENO_KV_ACCESS_TOKEN` - Used to authenticate against a KV Connect-compatible🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/self-hosting.md` around lines 16 - 17, Fix the typo in the description for the environment variable DENO_KV_ACCESS_TOKEN by changing "authenicate" to "authenticate" in the docs text where the variable is described; ensure the surrounding sentence remains grammatically correct and update any other occurrences of the misspelling in the same file or nearby lines if present.api/badges.ts-127-161 (1)
127-161:⚠️ Potential issue | 🟡 MinorSame missing return issue in
hcbTotalRaised.handleSimilar to
hcbBalanceOps, unhandled status codes will returnundefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/badges.ts` around lines 127 - 161, The handle method in hcbTotalRaised (override async handle(c: Context)) only returns responses for code == 200 and code == 404, leaving other getOrgData(result).code paths to return undefined; add a final fallback branch that handles all other status codes (e.g., else or default) and returns a proper SVG badge response or an error response so the function always returns a Response. Use the same pattern as the existing branches: build a badge via makeBadge (with resolveBadgeIcon("hcb-dark") and validateBadgeStyle(style) if needed), and return c.newResponse(badgeSvg, <appropriateStatus>, { "Content-Type": "image/svg+xml", "Cache-Control": "max-age=300" }) to ensure every execution path returns a response.docs/api-terms.md-34-34 (1)
34-34:⚠️ Potential issue | 🟡 MinorSpelling error: "Addeumn" should be "Addendum"
-## Addeumn: Unofficial Hack Club badges +## Addendum: Unofficial Hack Club badges🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api-terms.md` at line 34, Fix the spelling in the markdown header: change the header text "Addeumn: Unofficial Hack Club badges" to "Addendum: Unofficial Hack Club badges" so the section title uses the correct word ("Addendum").lib/githubAuth.ts-156-159 (1)
156-159:⚠️ Potential issue | 🟡 MinorTypo in documentation: "SHA-215" should be "SHA-512"
The JSDoc comment incorrectly states "SHA-215" when the implementation uses "SHA-512".
/** - * Hash a GitHub token into SHA-215 encoded text. + * Hash a GitHub token into SHA-512 encoded hex. * `@param` token Any string, but for this case, a GitHub token - * `@returns` + * `@returns` A hex-encoded SHA-512 hash of the token */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/githubAuth.ts` around lines 156 - 159, Update the JSDoc for the token hashing function in lib/githubAuth.ts to fix the typo: replace "SHA-215" with "SHA-512" so the documentation matches the implementation (the JSDoc block starting "Hash a GitHub token into SHA-215 encoded text." should read SHA-512).docs/api-terms.md-38-39 (1)
38-39:⚠️ Potential issue | 🟡 MinorSpelling error: "affliated" should be "affiliated"
-you agree to only use them in connection with your personal projects or hackathons and events affliated +you agree to only use them in connection with your personal projects or hackathons and events affiliated🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api-terms.md` around lines 38 - 39, Replace the misspelled word "affliated" with the correct spelling "affiliated" in the terms text (look for the sentence containing "you agree to only use them in connection with your personal projects or hackathons and events affliated with Hack Club HQ."); update that token so the sentence reads "...events affiliated with Hack Club HQ."docs/api-terms.md-15-17 (1)
15-17:⚠️ Potential issue | 🟡 MinorSpelling error: "stablize" should be "stabilize"
-We may change the API terms as we implement more features and stablize things at +We may change the API terms as we implement more features and stabilize things at🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api-terms.md` around lines 15 - 17, Fix the spelling typo in the API terms paragraph: replace the incorrect word "stablize" with "stabilize" in the sentence "We may change the API terms as we implement more features and stablize things..." in docs/api-terms.md; also scan the file for any other occurrences of "stablize" and correct them to "stabilize".api/admin.ts-133-143 (1)
133-143:⚠️ Potential issue | 🟡 MinorUnsafe access to error properties without type narrowing
Accessing
error.response.dataanderror.statuswithout checking iferrorhas these properties could throw aTypeErrorif the error is not an Octokit error (e.g., network failure).🛡️ Proposed fix with type narrowing
} catch (error) { + const ghError = error as { response?: { data?: unknown }; status?: number }; return c.json({ ok: false, error: { code: "GITHUB_API_ERROR", message: "Something went wrong while adding into the API admins team.", - ghApiResult: error.response.data, - ghApiStatusCode: error.status + ghApiResult: ghError.response?.data ?? null, + ghApiStatusCode: ghError.status ?? 500 } - }) + }, ghError.status ?? 500) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/admin.ts` around lines 133 - 143, The catch block in the API admin handler is unsafely accessing error.response.data and error.status; update the catch (error) handling in api/admin.ts (the block that returns the GITHUB_API_ERROR payload) to narrow and safely extract those fields—check if error is an object and has response and response.data (or use optional chaining like error?.response?.data) and a numeric status (or fallback to null/unknown), then populate ghApiResult and ghApiStatusCode with those safe values (and optionally include the raw error in a debug log) instead of directly accessing properties that may not exist.utils/manage-api-users.ts-104-105 (1)
104-105:⚠️ Potential issue | 🟡 MinorIncorrect type comparison: status code is a number, not a string
err.statusis a number, but comparing to"404"(string). This comparison will fail due to type mismatch even with loose equality in some cases.- if (err.status == "404") { + if (err.status === 404) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/manage-api-users.ts` around lines 104 - 105, The comparison in utils/manage-api-users.ts is using a string literal for HTTP status; change the check that currently inspects err.status == "404" to compare against a number (e.g. err.status === 404) or coerce the value to a number before comparing so the condition correctly detects a 404; update the branch that calls logger.warn(`user ${username} is not in organization`) to use the numeric comparison (or Number(err.status) === 404) so the warning fires as intended.lib/logos.ts-59-61 (1)
59-61:⚠️ Potential issue | 🟡 MinorIncorrect Error construction loses error details
throw new Error(error)coerces the error object to a string[object Object], losing the original error message and stack trace.🐛 Proposed fix
} catch (error) { - throw new Error(error) + throw error; }Or to add context:
} catch (error) { - throw new Error(error) + throw new Error(`Failed to lookup simple icon: ${iconName}`, { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/logos.ts` around lines 59 - 61, The catch block currently does "throw new Error(error)" which coerces the error object and loses message/stack; update the catch in lib/logos.ts to rethrow the original error (e.g., "throw error") or, if you need to add context, wrap it using Error's cause option (e.g., "throw new Error('failed to ...', { cause: error })") so the original error details are preserved for debugging.main.ts-37-41 (1)
37-41:⚠️ Potential issue | 🟡 MinorLoose equality comparison with potentially mismatched types
config.flags.edgeCache == trueuses loose equality. IfedgeCacheis the string"true"(common for env vars), this comparison will fail. Use strict equality with proper type coercion.🐛 Proposed fix
-if (Deno.env.get("DENO_DEPLOYMENT_ID") && Deno.env.get("DENO_REGION") || config.flags.edgeCache == true) { +if ((Deno.env.get("DENO_DEPLOYMENT_ID") && Deno.env.get("DENO_REGION")) || config.flags.edgeCache === true || config.flags.edgeCache === "true") {Or better, normalize the flag type in
lib/config.tsto always be boolean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.ts` around lines 37 - 41, The condition uses loose equality on config.flags.edgeCache which can be a string; update the check to use strict boolean logic or normalize the flag to a boolean in your config loader: either change the condition to use Boolean(config.flags.edgeCache) (or config.flags.edgeCache === true) when deciding to enable edge cache for app.get("/hcb/*", edgeCache(...)) and app.get("/badges/*", edgeCache(...)), or better convert config.flags.edgeCache to a real boolean in lib/config.ts so callers (like the if in main.ts) can simply test the boolean value.api/badges.ts-51-85 (1)
51-85:⚠️ Potential issue | 🟡 MinorMissing return statement for unhandled status codes
The
handlemethod only returns a response forcode == 200andcode == 404. Other status codes (e.g., 500, 403) will result inundefinedbeing returned, which may cause unexpected behavior.🛡️ Proposed fix to handle other status codes
} else if (code == 404) { const badgeSvg = makeBadge({ label: `HCB balance for unknown organization`, labelColor: "EC3750", message: `USD 0`, logoBase64: await resolveBadgeIcon("hcb-dark"), style: validateBadgeStyle(style) }); return c.newResponse(badgeSvg, 404, { "Content-Type": "image/svg+xml", "Cache-Control": "max-age=300", }); + } else { + const badgeSvg = makeBadge({ + label: "error", + message: "something went wrong", + color: "red", + style: validateBadgeStyle(style) + }); + return c.newResponse(badgeSvg, code, { + "Content-Type": "image/svg+xml", + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/badges.ts` around lines 51 - 85, The handle method in the class fails to return for status codes other than 200 and 404; update the method (inside handle and where getOrgData is called) to handle all other responses by returning a sensible response: create a default badge or JSON error response that includes the numeric code and a brief message, use the same headers ("Content-Type": "image/svg+xml", "Cache-Control": "max-age=300") if you return an SVG badge (or appropriate headers for JSON), and ensure every code path ends with return c.newResponse(..., code, headers). Locate the logic around getOrgData(...) and the existing if (code == 200) / else if (code == 404) branches and add a final else that constructs and returns the fallback response using the incoming style and resolveBadgeIcon("hcb-dark")/validateBadgeStyle as appropriate.api/badges.ts-88-88 (1)
88-88:⚠️ Potential issue | 🟡 MinorTypo in class name:
hcbTotalRasiedshould behcbTotalRaisedThe class name contains a spelling error that will propagate to the OpenAPI schema and any code referencing this class.
📝 Proposed fix
-export class hcbTotalRasied extends OpenAPIRoute { +export class hcbTotalRaised extends OpenAPIRoute {Also update the import in
main.ts:-import { generateSvg, hcbBalanceOps, hcbDonateButton, hcbTotalRasied } from "./api/badges.ts"; +import { generateSvg, hcbBalanceOps, hcbDonateButton, hcbTotalRaised } from "./api/badges.ts";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/badges.ts` at line 88, Rename the misspelled class hcbTotalRasied to hcbTotalRaised and update all references/imports accordingly (e.g., the import in main.ts) so the OpenAPI schema and any consumers use the corrected class name; ensure the exported name and any places that instantiate or import hcbTotalRasied are updated to hcbTotalRaised.lib/db.ts-107-109 (1)
107-109:⚠️ Potential issue | 🟡 MinorIncorrect Error construction and loss of stack trace
throw Error(error)should usenew Error(). Additionally, wrapping the error loses the original stack trace. Consider re-throwing directly or usingError.cause.🐛 Proposed fix
} catch (error) { - throw Error(error); + throw error; }Or if you want to add context:
} catch (error) { - throw Error(error); + throw new Error(`Failed to resolve badge icon: ${icon}`, { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/db.ts` around lines 107 - 109, In lib/db.ts replace the incorrect "throw Error(error)" in the catch block with either re-throwing the original error (throw error) to preserve the original stack, or if you need added context create a new Error using "new Error(message, { cause: error })" so the original error is preserved as the cause and the stack is retained; locate the catch block where Error(error) is called and apply one of these fixes.utils/manage-api-users.ts-154-155 (1)
154-155:⚠️ Potential issue | 🟡 MinorSame incorrect type comparison for status code
- if (err.status == "404") { + if (err.status === 404) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/manage-api-users.ts` around lines 154 - 155, The code is comparing err.status to the string "404" which is the wrong type; update the error check in the function handling API user membership (the block that currently reads if (err.status == "404") { logger.warn(`membership status: not in team or org`); }) to compare against a numeric status (e.g. err.status === 404) or normalize the value first (e.g. parseInt(err.status) === 404) and keep the existing logger.warn call; ensure you also handle other truthy/undefined cases safely by checking for err.status or err.statusCode consistently.
🧹 Nitpick comments (8)
utils/types.ts (1)
1-1: Use a type-only import forBadgeDatato reduce runtime coupling.
BadgeDatais used only as a type here; switching toimport typeavoids keeping an unnecessary value-level dependency onlib/db.ts.-import { BadgeData } from "../lib/db.ts"; +import type { BadgeData } from "../lib/db.ts";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/types.ts` at line 1, Replace the value-level import of BadgeData with a type-only import to avoid runtime coupling: change the import of BadgeData in utils/types.ts to a type import (use the symbol BadgeData and the module ../lib/db.ts) so the compiler treats it only as a type and does not retain a runtime dependency on lib/db.ts.api/admin.ts (2)
40-40: Use strict equality operator- if (parsedAuthHeader[1] == "null") { + if (parsedAuthHeader[1] === "null") {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/admin.ts` at line 40, The conditional in the admin auth check uses loose equality ("if (parsedAuthHeader[1] == \"null\")")—change it to strict equality using === to avoid type-coercion bugs; update the condition in the same auth-handling block (where parsedAuthHeader is inspected) to use "parsedAuthHeader[1] === 'null'".
147-157: Incomplete OpenAPIRoute implementationsThe
revokeAdminAccess,getAdminUserInfo, andlistAdminsclasses are empty stubs. Consider addingschemaandhandlemethods or marking them as not-implemented to avoid runtime errors if these routes are registered.Would you like me to generate skeleton implementations for these endpoints, or create GitHub issues to track this work?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/admin.ts` around lines 147 - 157, The three OpenAPIRoute classes revokeAdminAccess, getAdminUserInfo, and listAdmins are empty stubs and must either implement route metadata and handlers or be explicitly disabled; implement for each a static schema() method (HTTP method, path params/body/query schemas, responses) and an async handle(req, res) that validates input, calls the appropriate service (e.g., AdminService.revoke, AdminService.getInfo, AdminService.list), and returns proper HTTP status codes and error handling, or if you prefer to block registration throw/return a clear "NotImplemented" error in handle to prevent runtime routing errors; reference the class names revokeAdminAccess, getAdminUserInfo, and listAdmins when making the changes.lib/db.ts (2)
97-97: Missing semicolon in variable declaration- let result: string | null + let result: string | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/db.ts` at line 97, The variable declaration "let result: string | null" is missing a trailing semicolon; update the declaration of the variable named result (the line containing "let result: string | null") to include a semicolon at the end so the TypeScript syntax is correct.
98-99: Use strict equality when comparing withnullUse
===instead of==to avoid unexpected type coercion.- if (icon == null) { + if (icon === null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/db.ts` around lines 98 - 99, The comparison uses loose equality (icon == null) which allows type coercion; replace it with a strict check—either use icon === null if you only want to detect null, or explicitly check both absence cases with icon === null || icon === undefined to preserve the current semantics; update the conditional around the icon variable (the if handling the icon value) accordingly so behavior remains correct and uses strict equality.lib/githubAuth.ts (2)
49-49: Use strict equality operatorsUse
===and!==instead of==and!=to avoid type coercion issues.- if (cachedAuthData.value == null || forceCheck == true) { + if (cachedAuthData.value === null || forceCheck === true) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/githubAuth.ts` at line 49, Change the loose equality checks to strict ones in the conditional that reads "if (cachedAuthData.value == null || forceCheck == true)"; specifically use "===" and "!==" (e.g., check cachedAuthData.value === null or cachedAuthData.value === undefined as appropriate, and use forceCheck === true or simply forceCheck) so the condition avoids type coercion; update the conditional that references cachedAuthData.value and forceCheck accordingly.
32-34: Useconstfor variables that are never reassigned
now,ttl, andfiveMinsAgoare never reassigned after initialization.- let now = Date.now(); - let ttl = now + (5 * 60 * 1000); - let fiveMinsAgo = now - (5 * 60 * 1000); + const now = Date.now(); + const ttl = now + (5 * 60 * 1000); + const fiveMinsAgo = now - (5 * 60 * 1000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/githubAuth.ts` around lines 32 - 34, The variables now, ttl, and fiveMinsAgo in lib/githubAuth.ts are never reassigned; change their declarations from let to const to signal immutability and avoid accidental reassignment — update the lines that declare now, ttl, and fiveMinsAgo accordingly (e.g., replace "let now", "let ttl", "let fiveMinsAgo" with "const now", "const ttl", "const fiveMinsAgo").api/badges.ts (1)
203-229: Inconsistent error handling pattern forhcbDonateButtonLine 205 uses
apiReqData.querywithout the|| {}fallback that other handlers use. While this might work due to zod validation, it's inconsistent with the defensive pattern at lines 53 and 129. Additionally, the 404 case returnsc.notFound()which produces an HTML/text response instead of an SVG badge, breaking consistency with other endpoints.♻️ Suggested consistency improvements
override async handle(c: Context) { const apiReqData = await this.getValidatedData<typeof this.schema>(); - const { org, style } = apiReqData.query; + const { org, style } = apiReqData?.query || {}; const dbData = (await getBadgeData("hcb", "donate")).result?.data; const { result, code } = await getOrgData(org || "hq"); if (code == 200) { // ... existing code } else if (code == 404) { - return c.notFound(); + const badgeSvg = makeBadge({ + label: "HCB donate for unknown organization", + labelColor: "EC3750", + message: "not found", + logoBase64: await resolveBadgeIcon("hcb-dark"), + style: validateBadgeStyle(style) + }); + return c.newResponse(badgeSvg, 404, { + "Content-Type": "image/svg+xml", + "Cache-Control": "max-age=300", + }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/badges.ts` around lines 203 - 229, The handler's code in handle uses apiReqData.query directly (const { org, style } = apiReqData.query) and returns c.notFound() on a 404 which yields non-SVG output; change to destructure from a safe fallback (e.g., const { org, style } = apiReqData.query || {}) to match other handlers and avoid runtime errors, and replace the c.notFound() branch with returning a consistent SVG error badge (use makeBadge with an error Format and the same SVG headers and caching) so the endpoint always responds with an SVG; ensure you use the same helpers shown (getOrgData, makeBadge, validateBadgeStyle, resolveBadgeIcon) when constructing the error badge.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/self-hosting.md`:
- Around line 44-45: The docs incorrectly instruct users to run "deno run start"
even though "start" is defined as a task in deno.json; update the command in the
sentence referencing dotenvx so it uses "deno task start" (keeping the dotenvx
wrapper: dotenvx run -f .env.prod -- deno task start) and ensure the text still
instructs to point the reverse proxy to PORT; reference the deno.json "start"
task and the docs sentence that currently contains "deno run start" when making
the change.
In `@lib/githubAuth.ts`:
- Around line 10-11: The userAgent template literal contains invalid syntax
"$({config.homepage})" causing a runtime error; fix the userAgent property in
the GitHub auth setup by replacing that fragment with a proper template
placeholder for config.homepage (use ${config.homepage}) so the userAgent string
interpolates correctly; locate the userAgent key in lib/githubAuth.ts and update
the template literal that references config.homepage.
- Around line 161-165: hashToken is incorrectly passing the supplied token into
getValidatedGithubToken (which takes no args and reads GITHUB_TOKEN), causing
the wrong token to be hashed; update hashToken to use the function parameter
when present and fall back to getValidatedGithubToken() only when token is
undefined/empty, then pass that resulting token into
msgBuffer/crypto.subtle.digest and encodeHex (retain function names hashToken,
getValidatedGithubToken, msgBuffer, encodeHex).
In `@lib/logos.ts`:
- Around line 43-51: The function simpleIconLookup validates iconName
startsWith("si:") but passes the prefixed value to resolveSimpleIcon, causing
paths like icons/si:github.svg; fix by stripping the "si:" prefix before calling
resolveSimpleIcon (e.g., derive a strippedName from iconName such as
iconName.replace(/^si:/, "") or slice(3)) and pass strippedName to
resolveSimpleIcon while keeping the original iconName for the KV cache key
(cacheKv) and cacheBuster logic so cache lookup/storage behavior remains
unchanged.
- Around line 32-34: The Data URL is being built from decoded SVG text which
corrupts the base64 payload; instead either use the original base64 string from
response.data.content directly when forming the Data URL (replace
svgContentEncoded with response.data.content when creating result) or, if you
must modify the SVG, decode and then re-encode via Buffer to base64 before
embedding (ensure svgContentEncoded is a base64 string, not UTF-8). Update the
code around the svgContentEncoded/result variables in lib/logos.ts (where
response.data.content is read) accordingly.
In `@main.ts`:
- Around line 85-108: The middleware currently bypasses token validation for the
"/admin/auth-test" path by returning true inside the bearerAuth verifyToken
handler; remove that special-case so bearerAuth always calls handleGitHubAuth
(i.e., delete the if (c.req.path == "/admin/auth-test") return true branch) so
invalid/missing tokens trigger invalidTokenMessage, or if you must keep an
unauthenticated status endpoint implement it as a separate route (not inside
bearerAuth) or replace the bypass with a minimal check (e.g., verify an
Authorization header exists via c.req.headers.get("authorization") or call
handleGitHubAuth(token, true, forceCheckPerms) and return its result) to avoid
allowing unauthenticated requests to reach the handler.
In `@utils/badges-admin.ts`:
- Around line 105-107: BadgeData currently requires message and style but the
redirect payload only sets redirectUrl; change BadgeData into a discriminated
union (e.g., variants like { kind: 'redirect'; redirectUrl: string } | { kind:
'badge'; message: string; style: string; ... }) in lib/db.ts and update the
construction in utils/badges-admin.ts to create the redirect variant (include
the discriminant field, e.g., kind: 'redirect', and only redirectUrl), then
adjust any consumers to switch on the discriminant (kind) instead of assuming
message/style exist.
---
Outside diff comments:
In `@utils/badges-admin.ts`:
- Around line 101-115: When options.type === "redirect" abort early if
options.url is undefined by logging the error and returning/throwing before
building BadgeData or calling db.setBadgeData; ensure you reference options.url,
options.type and the BadgeData shape. After calling db.setBadgeData(project,
name, "redirect", data) check result.ok and on false log an error (including
result.error or result message) and exit non-zero (or throw) instead of silently
continuing; on true keep the existing logger.success path. This touches the
redirect branch in utils/badges-admin.ts, specifically the code around
db.setBadgeData, BadgeData, logger.error, and logger.success.
---
Minor comments:
In `@api/admin.ts`:
- Around line 133-143: The catch block in the API admin handler is unsafely
accessing error.response.data and error.status; update the catch (error)
handling in api/admin.ts (the block that returns the GITHUB_API_ERROR payload)
to narrow and safely extract those fields—check if error is an object and has
response and response.data (or use optional chaining like error?.response?.data)
and a numeric status (or fallback to null/unknown), then populate ghApiResult
and ghApiStatusCode with those safe values (and optionally include the raw error
in a debug log) instead of directly accessing properties that may not exist.
In `@api/badges.ts`:
- Around line 127-161: The handle method in hcbTotalRaised (override async
handle(c: Context)) only returns responses for code == 200 and code == 404,
leaving other getOrgData(result).code paths to return undefined; add a final
fallback branch that handles all other status codes (e.g., else or default) and
returns a proper SVG badge response or an error response so the function always
returns a Response. Use the same pattern as the existing branches: build a badge
via makeBadge (with resolveBadgeIcon("hcb-dark") and validateBadgeStyle(style)
if needed), and return c.newResponse(badgeSvg, <appropriateStatus>, {
"Content-Type": "image/svg+xml", "Cache-Control": "max-age=300" }) to ensure
every execution path returns a response.
- Around line 51-85: The handle method in the class fails to return for status
codes other than 200 and 404; update the method (inside handle and where
getOrgData is called) to handle all other responses by returning a sensible
response: create a default badge or JSON error response that includes the
numeric code and a brief message, use the same headers ("Content-Type":
"image/svg+xml", "Cache-Control": "max-age=300") if you return an SVG badge (or
appropriate headers for JSON), and ensure every code path ends with return
c.newResponse(..., code, headers). Locate the logic around getOrgData(...) and
the existing if (code == 200) / else if (code == 404) branches and add a final
else that constructs and returns the fallback response using the incoming style
and resolveBadgeIcon("hcb-dark")/validateBadgeStyle as appropriate.
- Line 88: Rename the misspelled class hcbTotalRasied to hcbTotalRaised and
update all references/imports accordingly (e.g., the import in main.ts) so the
OpenAPI schema and any consumers use the corrected class name; ensure the
exported name and any places that instantiate or import hcbTotalRasied are
updated to hcbTotalRaised.
In `@docs/api-terms.md`:
- Line 34: Fix the spelling in the markdown header: change the header text
"Addeumn: Unofficial Hack Club badges" to "Addendum: Unofficial Hack Club
badges" so the section title uses the correct word ("Addendum").
- Around line 38-39: Replace the misspelled word "affliated" with the correct
spelling "affiliated" in the terms text (look for the sentence containing "you
agree to only use them in connection with your personal projects or hackathons
and events affliated with Hack Club HQ."); update that token so the sentence
reads "...events affiliated with Hack Club HQ."
- Around line 15-17: Fix the spelling typo in the API terms paragraph: replace
the incorrect word "stablize" with "stabilize" in the sentence "We may change
the API terms as we implement more features and stablize things..." in
docs/api-terms.md; also scan the file for any other occurrences of "stablize"
and correct them to "stabilize".
In `@docs/README.md`:
- Around line 3-4: Fix the typo in the README intro sentence: change the word
"exisitng" to "existing" in the first paragraph ("Welcome to the Markdown
documentation for our Badges API! ... expand exisitng content.") so the sentence
reads "... expand existing content.".
In `@docs/self-hosting.md`:
- Around line 16-17: Fix the typo in the description for the environment
variable DENO_KV_ACCESS_TOKEN by changing "authenicate" to "authenticate" in the
docs text where the variable is described; ensure the surrounding sentence
remains grammatically correct and update any other occurrences of the
misspelling in the same file or nearby lines if present.
In `@lib/db.ts`:
- Around line 107-109: In lib/db.ts replace the incorrect "throw Error(error)"
in the catch block with either re-throwing the original error (throw error) to
preserve the original stack, or if you need added context create a new Error
using "new Error(message, { cause: error })" so the original error is preserved
as the cause and the stack is retained; locate the catch block where
Error(error) is called and apply one of these fixes.
In `@lib/githubAuth.ts`:
- Around line 156-159: Update the JSDoc for the token hashing function in
lib/githubAuth.ts to fix the typo: replace "SHA-215" with "SHA-512" so the
documentation matches the implementation (the JSDoc block starting "Hash a
GitHub token into SHA-215 encoded text." should read SHA-512).
In `@lib/logos.ts`:
- Around line 59-61: The catch block currently does "throw new Error(error)"
which coerces the error object and loses message/stack; update the catch in
lib/logos.ts to rethrow the original error (e.g., "throw error") or, if you need
to add context, wrap it using Error's cause option (e.g., "throw new
Error('failed to ...', { cause: error })") so the original error details are
preserved for debugging.
In `@lib/metadata.ts`:
- Line 11: Fix the typos in the public-facing metadata strings in
lib/metadata.ts: change "communties" to "communities" in the string "HCB badges
for use by organizations and communties fiscally sponsored by Hack Club through
the HCB platform." and change any "local host" occurrences to "localhost" (also
present elsewhere around the same file, e.g., the second occurrence near line
39). Update these literal strings so the exported metadata values (the
description entries) are corrected.
In `@main.ts`:
- Around line 37-41: The condition uses loose equality on config.flags.edgeCache
which can be a string; update the check to use strict boolean logic or normalize
the flag to a boolean in your config loader: either change the condition to use
Boolean(config.flags.edgeCache) (or config.flags.edgeCache === true) when
deciding to enable edge cache for app.get("/hcb/*", edgeCache(...)) and
app.get("/badges/*", edgeCache(...)), or better convert config.flags.edgeCache
to a real boolean in lib/config.ts so callers (like the if in main.ts) can
simply test the boolean value.
In `@README.md`:
- Around line 32-36: Update the README local/self-hosting text to fix two typos:
change "overriden" to "overridden" in the sentence mentioning PORT and change "a
more detailed instructions" to "more detailed instructions" in the "See the
docs" line (the line referencing "localhost:8080", "PORT", and
"./docs/self-hosting.md"). Ensure the corrected phrases appear exactly where
those original tokens are used.
In `@utils/manage-api-users.ts`:
- Around line 104-105: The comparison in utils/manage-api-users.ts is using a
string literal for HTTP status; change the check that currently inspects
err.status == "404" to compare against a number (e.g. err.status === 404) or
coerce the value to a number before comparing so the condition correctly detects
a 404; update the branch that calls logger.warn(`user ${username} is not in
organization`) to use the numeric comparison (or Number(err.status) === 404) so
the warning fires as intended.
- Around line 154-155: The code is comparing err.status to the string "404"
which is the wrong type; update the error check in the function handling API
user membership (the block that currently reads if (err.status == "404") {
logger.warn(`membership status: not in team or org`); }) to compare against a
numeric status (e.g. err.status === 404) or normalize the value first (e.g.
parseInt(err.status) === 404) and keep the existing logger.warn call; ensure you
also handle other truthy/undefined cases safely by checking for err.status or
err.statusCode consistently.
---
Nitpick comments:
In `@api/admin.ts`:
- Line 40: The conditional in the admin auth check uses loose equality ("if
(parsedAuthHeader[1] == \"null\")")—change it to strict equality using === to
avoid type-coercion bugs; update the condition in the same auth-handling block
(where parsedAuthHeader is inspected) to use "parsedAuthHeader[1] === 'null'".
- Around line 147-157: The three OpenAPIRoute classes revokeAdminAccess,
getAdminUserInfo, and listAdmins are empty stubs and must either implement route
metadata and handlers or be explicitly disabled; implement for each a static
schema() method (HTTP method, path params/body/query schemas, responses) and an
async handle(req, res) that validates input, calls the appropriate service
(e.g., AdminService.revoke, AdminService.getInfo, AdminService.list), and
returns proper HTTP status codes and error handling, or if you prefer to block
registration throw/return a clear "NotImplemented" error in handle to prevent
runtime routing errors; reference the class names revokeAdminAccess,
getAdminUserInfo, and listAdmins when making the changes.
In `@api/badges.ts`:
- Around line 203-229: The handler's code in handle uses apiReqData.query
directly (const { org, style } = apiReqData.query) and returns c.notFound() on a
404 which yields non-SVG output; change to destructure from a safe fallback
(e.g., const { org, style } = apiReqData.query || {}) to match other handlers
and avoid runtime errors, and replace the c.notFound() branch with returning a
consistent SVG error badge (use makeBadge with an error Format and the same SVG
headers and caching) so the endpoint always responds with an SVG; ensure you use
the same helpers shown (getOrgData, makeBadge, validateBadgeStyle,
resolveBadgeIcon) when constructing the error badge.
In `@lib/db.ts`:
- Line 97: The variable declaration "let result: string | null" is missing a
trailing semicolon; update the declaration of the variable named result (the
line containing "let result: string | null") to include a semicolon at the end
so the TypeScript syntax is correct.
- Around line 98-99: The comparison uses loose equality (icon == null) which
allows type coercion; replace it with a strict check—either use icon === null if
you only want to detect null, or explicitly check both absence cases with icon
=== null || icon === undefined to preserve the current semantics; update the
conditional around the icon variable (the if handling the icon value)
accordingly so behavior remains correct and uses strict equality.
In `@lib/githubAuth.ts`:
- Line 49: Change the loose equality checks to strict ones in the conditional
that reads "if (cachedAuthData.value == null || forceCheck == true)";
specifically use "===" and "!==" (e.g., check cachedAuthData.value === null or
cachedAuthData.value === undefined as appropriate, and use forceCheck === true
or simply forceCheck) so the condition avoids type coercion; update the
conditional that references cachedAuthData.value and forceCheck accordingly.
- Around line 32-34: The variables now, ttl, and fiveMinsAgo in
lib/githubAuth.ts are never reassigned; change their declarations from let to
const to signal immutability and avoid accidental reassignment — update the
lines that declare now, ttl, and fiveMinsAgo accordingly (e.g., replace "let
now", "let ttl", "let fiveMinsAgo" with "const now", "const ttl", "const
fiveMinsAgo").
In `@utils/types.ts`:
- Line 1: Replace the value-level import of BadgeData with a type-only import to
avoid runtime coupling: change the import of BadgeData in utils/types.ts to a
type import (use the symbol BadgeData and the module ../lib/db.ts) so the
compiler treats it only as a type and does not retain a runtime dependency on
lib/db.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3b6ceefb-8ba2-4d12-9e71-50730bd83768
📒 Files selected for processing (21)
.vscode/settings.jsonCONTRIBUTING.mdREADME.mdapi/admin.tsapi/badges.tsdocs/README.mddocs/api-terms.mddocs/deno-scripts.mddocs/self-hosting.mdlib/config.tslib/db.tslib/githubAuth.tslib/hcb.tslib/logos.tslib/metadata.tslib/utils.tsmain.tsutils/badges-admin.tsutils/manage-api-users.tsutils/preseed-kv.tsutils/types.ts
✅ Files skipped from review due to trivial changes (2)
- lib/hcb.ts
- CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/utils.ts
- lib/config.ts
- .vscode/settings.json
| 6. Otherwise, just run `dotenvx run -f .env.prod -- deno run start` and point your reverse | ||
| proxy to `PORT`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether start/dev are defined as tasks and compare doc commands.
fd 'deno.json$' -x sh -c 'echo ">>> $1"; rg -n "\"scripts\"|\"start\"|\"dev\"|\"deploy\"" "$1"'
rg -n -C2 'deno run start|deno task start|deno task dev' docs/self-hosting.md docs/deno-scripts.md README.mdRepository: lorebooks-wiki/badges-api
Length of output: 639
🏁 Script executed:
fd -type f 'deno\.json' -o -name 'deno\.jsonc'Repository: lorebooks-wiki/badges-api
Length of output: 239
🏁 Script executed:
rg -n '\"start\"' --type jsonRepository: lorebooks-wiki/badges-api
Length of output: 144
🏁 Script executed:
fd --type f 'deno\.json'Repository: lorebooks-wiki/badges-api
Length of output: 78
🏁 Script executed:
rg 'start' --type json -A 2 -B 2Repository: lorebooks-wiki/badges-api
Length of output: 557
Line 44 uses the wrong Deno command (deno run start).
Since start is defined as a task in deno.json, the correct command is deno task start. Otherwise, the command will fail for self-hosters.
Suggested fix
-6. Otherwise, just run `dotenvx run -f .env.prod -- deno run start` and point your reverse
+6. Otherwise, just run `dotenvx run -f .env.prod -- deno task start` and point your reverse📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 6. Otherwise, just run `dotenvx run -f .env.prod -- deno run start` and point your reverse | |
| proxy to `PORT`. | |
| 6. Otherwise, just run `dotenvx run -f .env.prod -- deno task start` and point your reverse | |
| proxy to `PORT`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/self-hosting.md` around lines 44 - 45, The docs incorrectly instruct
users to run "deno run start" even though "start" is defined as a task in
deno.json; update the command in the sentence referencing dotenvx so it uses
"deno task start" (keeping the dotenvx wrapper: dotenvx run -f .env.prod -- deno
task start) and ensure the text still instructs to point the reverse proxy to
PORT; reference the deno.json "start" task and the docs sentence that currently
contains "deno run start" when making the change.
| userAgent: `@lorebooks-wiki/badges-api $({config.homepage})` | ||
| }) |
There was a problem hiding this comment.
Syntax error in userAgent string template literal
The string template uses $({config.homepage}) which is invalid syntax. This will cause a runtime error.
🐛 Proposed fix
export const github = (token: string) => new Octokit({
auth: token,
- userAgent: `@lorebooks-wiki/badges-api $({config.homepage})`
+ userAgent: `@lorebooks-wiki/badges-api (${config.homepage})`
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| userAgent: `@lorebooks-wiki/badges-api $({config.homepage})` | |
| }) | |
| userAgent: `@lorebooks-wiki/badges-api (${config.homepage})` | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/githubAuth.ts` around lines 10 - 11, The userAgent template literal
contains invalid syntax "$({config.homepage})" causing a runtime error; fix the
userAgent property in the GitHub auth setup by replacing that fragment with a
proper template placeholder for config.homepage (use ${config.homepage}) so the
userAgent string interpolates correctly; locate the userAgent key in
lib/githubAuth.ts and update the template literal that references
config.homepage.
| export async function hashToken(token: string) { | ||
| const validatedToken = getValidatedGithubToken(token) | ||
| const msg = msgBuffer(validatedToken); | ||
| const hashBuff = await crypto.subtle.digest("SHA-512", msg); | ||
| return encodeHex(hashBuff); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify getValidatedGithubToken signature in lib/config.ts
ast-grep --pattern 'export function getValidatedGithubToken($$$)'Repository: lorebooks-wiki/badges-api
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for getValidatedGithubToken function definition
rg "getValidatedGithubToken" --max-count 20Repository: lorebooks-wiki/badges-api
Length of output: 352
🏁 Script executed:
#!/bin/bash
# List files in lib directory to understand structure
ls -la lib/Repository: lorebooks-wiki/badges-api
Length of output: 643
🏁 Script executed:
#!/bin/bash
# Check if config.ts exists and read it
fd "config.ts" -t fRepository: lorebooks-wiki/badges-api
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Read lib/config.ts to see the actual function implementation
cat -n lib/config.tsRepository: lorebooks-wiki/badges-api
Length of output: 1483
🏁 Script executed:
#!/bin/bash
# Read lib/githubAuth.ts focusing on the hashToken function around line 161-165
sed -n '155,170p' lib/githubAuth.tsRepository: lorebooks-wiki/badges-api
Length of output: 443
getValidatedGithubToken does not accept parameters—hashing the wrong token
getValidatedGithubToken() is defined with no parameters and reads from the GITHUB_TOKEN environment variable. Calling it as getValidatedGithubToken(token) ignores the supplied token and hashes the service token instead, breaking authentication.
🐛 Proposed fix
export async function hashToken(token: string) {
- const validatedToken = getValidatedGithubToken(token)
- const msg = msgBuffer(validatedToken);
+ const msg = msgBuffer(token);
const hashBuff = await crypto.subtle.digest("SHA-512", msg);
return encodeHex(hashBuff);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function hashToken(token: string) { | |
| const validatedToken = getValidatedGithubToken(token) | |
| const msg = msgBuffer(validatedToken); | |
| const hashBuff = await crypto.subtle.digest("SHA-512", msg); | |
| return encodeHex(hashBuff); | |
| export async function hashToken(token: string) { | |
| const msg = msgBuffer(token); | |
| const hashBuff = await crypto.subtle.digest("SHA-512", msg); | |
| return encodeHex(hashBuff); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/githubAuth.ts` around lines 161 - 165, hashToken is incorrectly passing
the supplied token into getValidatedGithubToken (which takes no args and reads
GITHUB_TOKEN), causing the wrong token to be hashed; update hashToken to use the
function parameter when present and fall back to getValidatedGithubToken() only
when token is undefined/empty, then pass that resulting token into
msgBuffer/crypto.subtle.digest and encodeHex (retain function names hashToken,
getValidatedGithubToken, msgBuffer, encodeHex).
| const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8"); | ||
| const result = `data:image/svg+xml;base64,${svgContentEncoded}`; | ||
| return result |
There was a problem hiding this comment.
Incorrect base64 encoding produces corrupted Data URL
response.data.content from GitHub API is already base64-encoded. The current code decodes it to UTF-8 SVG text, then embeds that raw text in the Data URL without re-encoding it. This produces an invalid Data URL.
Either keep the original base64 content directly, or re-encode after decoding:
🐛 Option 1: Use the original base64 directly (simpler)
- const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8");
- const result = `data:image/svg+xml;base64,${svgContentEncoded}`;
+ // content is already base64-encoded from GitHub API
+ const base64Content = response.data.content.replace(/\n/g, ''); // Remove any line breaks
+ const result = `data:image/svg+xml;base64,${base64Content}`;🐛 Option 2: Decode then re-encode (if you need to process the SVG)
- const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8");
- const result = `data:image/svg+xml;base64,${svgContentEncoded}`;
+ const svgContent = Buffer.from(response.data.content, "base64").toString("utf-8");
+ const reEncoded = Buffer.from(svgContent, "utf-8").toString("base64");
+ const result = `data:image/svg+xml;base64,${reEncoded}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8"); | |
| const result = `data:image/svg+xml;base64,${svgContentEncoded}`; | |
| return result | |
| const base64Content = response.data.content.replace(/\n/g, ''); | |
| const result = `data:image/svg+xml;base64,${base64Content}`; | |
| return result |
| const svgContentEncoded = Buffer.from(response.data.content, "base64").toString("utf-8"); | |
| const result = `data:image/svg+xml;base64,${svgContentEncoded}`; | |
| return result | |
| const svgContent = Buffer.from(response.data.content, "base64").toString("utf-8"); | |
| const reEncoded = Buffer.from(svgContent, "utf-8").toString("base64"); | |
| const result = `data:image/svg+xml;base64,${reEncoded}`; | |
| return result |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/logos.ts` around lines 32 - 34, The Data URL is being built from decoded
SVG text which corrupts the base64 payload; instead either use the original
base64 string from response.data.content directly when forming the Data URL
(replace svgContentEncoded with response.data.content when creating result) or,
if you must modify the SVG, decode and then re-encode via Buffer to base64
before embedding (ensure svgContentEncoded is a base64 string, not UTF-8).
Update the code around the svgContentEncoded/result variables in lib/logos.ts
(where response.data.content is read) accordingly.
| if (!iconName.startsWith("si:")) { | ||
| throw new Error("not a simple-icon icon name") | ||
| } | ||
| } | ||
|
|
||
| return svg2base64(iconSvg); | ||
| } | ||
|
|
||
| function prepareNamedLogo({ name, color, style, size }) { | ||
| if (typeof name !== "string") { | ||
| return undefined; | ||
| } | ||
| // check on our KV cache first | ||
| const cacheKv = await kvApi.get<string>(["badgeIcons", iconName]) | ||
|
|
||
| name = name.toLowerCase(); | ||
| // if not cached OR the cache busting is enabled, run the resolver | ||
| if (cacheKv.value === null || cacheBuster === true) { | ||
| const upstreamData = await resolveSimpleIcon(iconName) |
There was a problem hiding this comment.
si: prefix not stripped before calling resolveSimpleIcon
simpleIconLookup validates that iconName starts with si: but passes the full name (including prefix) to resolveSimpleIcon. This results in an invalid path like icons/si:github.svg instead of icons/github.svg.
🐛 Proposed fix
if (!iconName.startsWith("si:")) {
throw new Error("not a simple-icon icon name")
}
+ // Strip the "si:" prefix for the actual icon name
+ const actualIconName = iconName.replace(/^si:/, '');
// check on our KV cache first
const cacheKv = await kvApi.get<string>(["badgeIcons", iconName])
// if not cached OR the cache busting is enabled, run the resolver
if (cacheKv.value === null || cacheBuster === true) {
- const upstreamData = await resolveSimpleIcon(iconName)
+ const upstreamData = await resolveSimpleIcon(actualIconName)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!iconName.startsWith("si:")) { | |
| throw new Error("not a simple-icon icon name") | |
| } | |
| } | |
| return svg2base64(iconSvg); | |
| } | |
| function prepareNamedLogo({ name, color, style, size }) { | |
| if (typeof name !== "string") { | |
| return undefined; | |
| } | |
| // check on our KV cache first | |
| const cacheKv = await kvApi.get<string>(["badgeIcons", iconName]) | |
| name = name.toLowerCase(); | |
| // if not cached OR the cache busting is enabled, run the resolver | |
| if (cacheKv.value === null || cacheBuster === true) { | |
| const upstreamData = await resolveSimpleIcon(iconName) | |
| if (!iconName.startsWith("si:")) { | |
| throw new Error("not a simple-icon icon name") | |
| } | |
| // Strip the "si:" prefix for the actual icon name | |
| const actualIconName = iconName.replace(/^si:/, ''); | |
| // check on our KV cache first | |
| const cacheKv = await kvApi.get<string>(["badgeIcons", iconName]) | |
| // if not cached OR the cache busting is enabled, run the resolver | |
| if (cacheKv.value === null || cacheBuster === true) { | |
| const upstreamData = await resolveSimpleIcon(actualIconName) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/logos.ts` around lines 43 - 51, The function simpleIconLookup validates
iconName startsWith("si:") but passes the prefixed value to resolveSimpleIcon,
causing paths like icons/si:github.svg; fix by stripping the "si:" prefix before
calling resolveSimpleIcon (e.g., derive a strippedName from iconName such as
iconName.replace(/^si:/, "") or slice(3)) and pass strippedName to
resolveSimpleIcon while keeping the original iconName for the KV cache key
(cacheKv) and cacheBuster logic so cache lookup/storage behavior remains
unchanged.
| app.use("/admin/*", bearerAuth({ | ||
| verifyToken: async (token: string, c: Context) => { | ||
| const forceCheckPerms = Boolean(c.req.query("force")) || false | ||
| if (c.req.path == "/admin/auth-test") { | ||
| return true // skip validating the token on auth-test endpoint | ||
| } else { | ||
| return await handleGitHubAuth(token, true, forceCheckPerms) | ||
| } | ||
| }, | ||
| invalidAuthenticationHeaderMessage: { | ||
| ok: false, | ||
| error: { | ||
| code: "MISSING_AUTH", | ||
| message: "Authorization header is required" | ||
| } | ||
| }, | ||
| invalidTokenMessage: { | ||
| ok: false, | ||
| error: { | ||
| code: "AUTH_ERROR", | ||
| message: "Your API token is invalid" | ||
| } | ||
| } | ||
| })) |
There was a problem hiding this comment.
Security concern: /admin/auth-test bypasses bearer auth validation
Returning true unconditionally at line 89 allows any request (including invalid tokens or no token) to reach the /admin/auth-test handler. While the handler performs its own validation, this:
- Bypasses the middleware's
invalidTokenMessageresponse for this endpoint - Allows unauthenticated requests to consume handler resources and potentially trigger GitHub API calls
Consider removing the bypass or using a lighter validation check.
🛡️ Suggested approach
Option 1: Remove the bypass entirely - let the middleware validate:
verifyToken: async (token: string, c: Context) => {
const forceCheckPerms = Boolean(c.req.query("force")) || false
- if (c.req.path == "/admin/auth-test") {
- return true // skip validating the token on auth-test endpoint
- } else {
- return await handleGitHubAuth(token, true, forceCheckPerms)
- }
+ return await handleGitHubAuth(token, true, forceCheckPerms)
},Option 2: If you want auth-test to show auth status for any token (even invalid), consider a separate non-authenticated endpoint.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app.use("/admin/*", bearerAuth({ | |
| verifyToken: async (token: string, c: Context) => { | |
| const forceCheckPerms = Boolean(c.req.query("force")) || false | |
| if (c.req.path == "/admin/auth-test") { | |
| return true // skip validating the token on auth-test endpoint | |
| } else { | |
| return await handleGitHubAuth(token, true, forceCheckPerms) | |
| } | |
| }, | |
| invalidAuthenticationHeaderMessage: { | |
| ok: false, | |
| error: { | |
| code: "MISSING_AUTH", | |
| message: "Authorization header is required" | |
| } | |
| }, | |
| invalidTokenMessage: { | |
| ok: false, | |
| error: { | |
| code: "AUTH_ERROR", | |
| message: "Your API token is invalid" | |
| } | |
| } | |
| })) | |
| app.use("/admin/*", bearerAuth({ | |
| verifyToken: async (token: string, c: Context) => { | |
| const forceCheckPerms = Boolean(c.req.query("force")) || false | |
| return await handleGitHubAuth(token, true, forceCheckPerms) | |
| }, | |
| invalidAuthenticationHeaderMessage: { | |
| ok: false, | |
| error: { | |
| code: "MISSING_AUTH", | |
| message: "Authorization header is required" | |
| } | |
| }, | |
| invalidTokenMessage: { | |
| ok: false, | |
| error: { | |
| code: "AUTH_ERROR", | |
| message: "Your API token is invalid" | |
| } | |
| } | |
| })) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.ts` around lines 85 - 108, The middleware currently bypasses token
validation for the "/admin/auth-test" path by returning true inside the
bearerAuth verifyToken handler; remove that special-case so bearerAuth always
calls handleGitHubAuth (i.e., delete the if (c.req.path == "/admin/auth-test")
return true branch) so invalid/missing tokens trigger invalidTokenMessage, or if
you must keep an unauthenticated status endpoint implement it as a separate
route (not inside bearerAuth) or replace the bypass with a minimal check (e.g.,
verify an Authorization header exists via c.req.headers.get("authorization") or
call handleGitHubAuth(token, true, forceCheckPerms) and return its result) to
avoid allowing unauthenticated requests to reach the handler.
| const data: BadgeData = { | ||
| redirectUrl: options.url, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Inspect BadgeData shape
rg -n -A20 -B2 '^export type BadgeData' lib/db.ts
# 2) Inspect setBadgeData signature
rg -n -A12 -B2 '^export async function setBadgeData' lib/db.ts
# 3) Inspect redirect payload assignment in CLI
rg -n -A10 -B4 'if \(options\.type == "redirect"\)' utils/badges-admin.tsRepository: lorebooks-wiki/badges-api
Length of output: 1677
BadgeData type mismatch: required message and style fields missing for redirect payload.
The BadgeData type in lib/db.ts requires message and style fields, but the redirect payload at lines 105-107 only provides redirectUrl. This creates a type-model mismatch. Consider using discriminated union types to have separate payload shapes for redirect vs. badge types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/badges-admin.ts` around lines 105 - 107, BadgeData currently requires
message and style but the redirect payload only sets redirectUrl; change
BadgeData into a discriminated union (e.g., variants like { kind: 'redirect';
redirectUrl: string } | { kind: 'badge'; message: string; style: string; ... })
in lib/db.ts and update the construction in utils/badges-admin.ts to create the
redirect variant (include the discriminant field, e.g., kind: 'redirect', and
only redirectUrl), then adjust any consumers to switch on the discriminant
(kind) instead of assuming message/style exist.
TODO Update this placeholder with some real description soon.
This change is
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Style