Add Admin API to the backend + more docs and badges #1
Conversation
…hed requests Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
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) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Also updated the secrets behind the scenes rn, alongside some OpenAPI metadata chaos updates. Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications across various files, primarily focusing on adding new environment variables, enhancing API functionality, and improving documentation clarity. Key changes include the addition of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant GitHub
participant Cache
Client->>API: Request authentication
API->>GitHub: Verify token
GitHub-->>API: Return user data
API->>Cache: Store user data
Cache-->>API: Confirmation
API-->>Client: Return authentication result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@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.
-[email protected]` or file a new issue here. +[email protected]` 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) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Also finalize self-hosting docs ftw Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
…for Deno.KV.get Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
…ions Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
Signed-off-by: Andrei Jiroh Halili (RecapTime.dev) <[email protected]>
|
I might ship this with just the barebones for now and implement the rest of the API later. |
TODO Update this placeholder with some real description soon.
This change is
Summary by CodeRabbit
Release Notes
New Features
GITHUB_TOKENfor enhanced configuration.Documentation
Bug Fixes
Style