Implement Account Export API endpoint #33
Conversation
WalkthroughThe changes in this pull request introduce a new POST endpoint at Changes
Possibly related PRs
Suggested reviewers
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
src/api/v2/index.ts (2)
32-33: Enhance logging for better traceabilityWhile basic logging is implemented, it could be more informative to aid in debugging and monitoring.
Enhance the logging to include more context:
- logger.info("Received account export request"); + logger.info("Received account export request for actor: {actor}", { actor: c.req.param("actor") });Consider adding more log entries at key points in the export process, such as when the export starts, completes, or encounters issues.
31-36: Summary: Incomplete implementation of account export endpointThe current implementation of the account export endpoint is a good starting point but requires significant work to meet the PR objectives and follow best practices. Key areas for improvement include:
- Implementing authentication and authorization
- Developing the core account export functionality
- Adding error handling and input validation
- Enhancing logging for better traceability
- Updating the response to return the exported data as a .tar file
Please address these points to ensure a robust and functional account export feature.
Consider breaking down the export functionality into smaller, reusable functions. This will improve maintainability and make unit testing easier. For example:
validateActor(actor: string): booleanexportActorProfile(actor: string): Promise<ExportData>createTarStream(exportData: ExportData): Promise<ReadStream>This modular approach will allow for better separation of concerns and easier testing of each component.
dmitrizagidulin
force-pushed
the
account-export
branch
from
f802d1f to
9a51cd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (2)
src/entities/exportAccount.ts (2)
12-12: EnsurehomeUrlends with a trailing slash to prevent URL concatenation issuesTo avoid issues when concatenating URLs (such as missing or double slashes), ensure that
homeUrlends with a single trailing slash. You might normalizehomeUrlor use URL utilities to properly construct URLs.
103-108: Remove debuggingconsole.logstatements from production codeThe
console.logstatements used for debugging should be removed or replaced with proper logging mechanisms to avoid unnecessary console output in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
package.json(1 hunks)src/api/v2/controllers/accountExport.ts(1 hunks)src/api/v2/index.ts(2 hunks)src/components/AccountList.tsx(3 hunks)src/entities/exportAccount.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/v2/index.ts
🔇 Additional comments (2)
package.json (1)
23-23: Verify the new dependency for security vulnerabilities and compatibility
Ensure that the newly added dependency "@interop/wallet-export-ts": "^0.1.1" is up-to-date and free from known vulnerabilities. Check for any security advisories related to this package.
Run the following script to check for security advisories and the latest version:
✅ Verification successful
Update dependency version to latest release
The package @interop/wallet-export-ts has a newer version (0.1.3) available. While no security vulnerabilities were found, it's recommended to update to the latest version for potential bug fixes and improvements. Update the version in package.json from ^0.1.1 to ^0.1.3.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for security advisories and latest version of '@interop/wallet-export-ts' package.
# Fetch the latest version from npm registry
curl -s https://registry.npmjs.org/@interop/wallet-export-ts | jq -r '."dist-tags".latest'
# Check for known vulnerabilities using GitHub's security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@interop/wallet-export-ts") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 457
src/components/AccountList.tsx (1)
78-78: Good job updating inline styles to use React's style objects
Converting the style prop to use object notation { display: 'block' } aligns with React's recommended practices for inline styles.
Also applies to: 86-86
src/entities/exportAccount.ts
Outdated
| // @ts-ignore-next-line | ||
| import { exportActorProfile } from "@interop/wallet-export-ts"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid suppressing TypeScript errors with @ts-ignore; resolve the type issues instead
Using @ts-ignore suppresses TypeScript errors, potentially hiding important type safety issues. Please address the underlying TypeScript error caused when importing exportActorProfile from @interop/wallet-export-ts by updating type definitions or ensuring correct module resolution.
src/entities/exportAccount.ts
Outdated
| async loadFollows(type: "following" | "followers") { | ||
| const column = type === "following" ? "followerId" : "followingId"; | ||
| return db.query.follows.findMany({ | ||
| where: eq(schema.follows[column], this.actorId), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential logical error in loadFollows; verify column selection
In the loadFollows method, the selection of the column in line 37 may be reversed. When type is "following", using "followerId" might not retrieve the correct records. Please verify that the correct columns are used to fetch the intended follow relationships.
src/entities/exportAccount.ts
Outdated
| // @ts-ignore-next-line | ||
| return c.body(exportTarballStream, 200, { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid suppressing TypeScript errors with @ts-ignore; resolve the type issues instead
Using @ts-ignore here hides potential type errors in the c.body method. Consider providing the correct type annotations or updating the code to address the TypeScript issue.
src/entities/exportAccount.ts
Outdated
| "Content-Type": "application/x-tar", | ||
| "Content-Disposition": `attachment; filename="account_export_${this.actorId}.tar"`, | ||
| }); |
There was a problem hiding this comment.
Sanitize actorId in Content-Disposition header to prevent header injection
Directly using actorId in the Content-Disposition header can lead to header injection vulnerabilities if actorId contains special characters. Sanitize or encode actorId before including it in the header to ensure safety.
Apply this change to sanitize actorId:
-"Content-Disposition": `attachment; filename="account_export_${this.actorId}.tar"`,
+const sanitizedActorId = encodeURIComponent(this.actorId);
+"Content-Disposition": `attachment; filename="account_export_${sanitizedActorId}.tar"`,📝 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.
| "Content-Type": "application/x-tar", | |
| "Content-Disposition": `attachment; filename="account_export_${this.actorId}.tar"`, | |
| }); | |
| "Content-Type": "application/x-tar", | |
| const sanitizedActorId = encodeURIComponent(this.actorId); | |
| "Content-Disposition": `attachment; filename="account_export_${sanitizedActorId}.tar"`, | |
| }); |
| if (owner.handle !== actorId) { | ||
| return c.json({ error: "Forbidden" }, 403); | ||
| } |
There was a problem hiding this comment.
Verify authorization logic; ensure correct comparison for ownership
Comparing owner.handle to actorId may not correctly verify the account ownership, especially if actorId represents an ID rather than a handle. Consider comparing owner.id to actorId to accurately confirm the user's authorization.
src/components/AccountList.tsx
Outdated
| {/* Export Account Button */} | ||
| <form action={`/api/v2/${account.id}/accountExport`} method='post' > | ||
| <button type='submit'>Export Account</button> | ||
| </form> |
There was a problem hiding this comment.
Implement CSRF protection for the account export form
The POST form for exporting the account should include CSRF protection to prevent cross-site request forgery attacks. Consider adding a CSRF token to the form and validating it on the server side.
|
(we're going through to address @coderabbitai's comments) |
|
Tip For best results, initiate chat on the files or code changes. @dmitrizagidulin: Great! Let me know if you have any questions or need further assistance. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
src/entities/exportAccount.ts (2)
10-11: Consider using a more robust environment variable configuration.The current environment variable handling could be improved for better maintainability and type safety.
Consider using a dedicated configuration module:
-// biome-ignore lint/complexity/useLiteralKeys: <explanation> -const homeUrl = process.env["HOME_URL"] || "http://localhost:3000/"; +import { z } from "zod"; + +const envSchema = z.object({ + HOME_URL: z.string().url().default("http://localhost:3000/") +}); + +const env = envSchema.parse(process.env); +const homeUrl = env.HOME_URL;
102-107: Remove debug console.log statements.Debug logging statements should be removed before merging.
- console.log("🚀 ~ AccountExporter ~ exportData ~ lists:", lists); const serializedLists = lists.map((list) => serializeList(list)); - console.log( - "🚀 ~ AccountExporter ~ exportData ~ serializedLists:", - serializedLists, - );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (3)
package.json(1 hunks)src/api/v2/controllers/accountExport.ts(1 hunks)src/entities/exportAccount.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/api/v2/controllers/accountExport.ts
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
src/entities/exportAccount.ts
[error] 134-134: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (2)
src/entities/exportAccount.ts (2)
13-19: LGTM! Clean and focused class implementation.
21-52: LGTM! Well-structured database queries with proper relations.
The data loading methods are implemented correctly, including the column selection in loadFollows. The method appropriately uses:
followingIdfor finding accounts that the actor followsfollowerIdfor finding accounts that follow the actor
src/entities/exportAccount.ts
Outdated
| bookmarks: serializedBookmarks, | ||
| }); | ||
|
|
||
| return c.body(exportTarballStream as any, 200, { |
There was a problem hiding this comment.
Replace any type assertion with proper typing.
Using any disables type checking. The type of exportTarballStream should be properly defined.
- return c.body(exportTarballStream as any, 200, {
+ import type { Readable } from 'stream';
+ return c.body(exportTarballStream as Readable, 200, {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 134-134: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
src/entities/exportAccount.ts
Outdated
| async exportData(c: Context) { | ||
| const account = await this.loadAccount(); | ||
| if (!account) return c.json({ error: "Actor not found" }, 404); | ||
|
|
||
| // Load and serialize posts | ||
| const postsData = await this.loadPosts(); | ||
| const serializedPosts = postsData.map((post) => | ||
| serializePost(post, { id: account.owner.id }, c.req.url), | ||
| ); | ||
|
|
||
| // Load and serialize lists | ||
| const lists = await this.loadLists(); | ||
| console.log("🚀 ~ AccountExporter ~ exportData ~ lists:", lists); | ||
| const serializedLists = lists.map((list) => serializeList(list)); | ||
| console.log( | ||
| "🚀 ~ AccountExporter ~ exportData ~ serializedLists:", | ||
| serializedLists, | ||
| ); | ||
|
|
||
| // Load and serialize followers | ||
| const followers = await this.loadFollows("followers"); | ||
| const serializedFollowers = this.serializeFollowers(followers); | ||
|
|
||
| // Load and serialize following | ||
| const followingAccounts = await this.loadFollows("following"); | ||
| const serializedFollowing = this.serializeFollowing(followingAccounts); | ||
|
|
||
| // Load and serialize bookmarks | ||
| const bookmarks = await this.loadBookmarks(); | ||
| const serializedBookmarks = this.serializeBookmarks(bookmarks); | ||
|
|
||
| // Generate export tarball | ||
| const exportTarballStream = exportActorProfile({ | ||
| actorProfile: serializeAccount( | ||
| { ...account, successor: null }, | ||
| c.req.url, | ||
| ), | ||
| outbox: serializedPosts, | ||
| lists: serializedLists, | ||
| followers: serializedFollowers, | ||
| followingAccounts: serializedFollowing, | ||
| bookmarks: serializedBookmarks, | ||
| }); | ||
|
|
||
| return c.body(exportTarballStream as any, 200, { | ||
| "Content-Type": "application/x-tar", | ||
| "Content-Disposition": `attachment; filename="account_export_${encodeURIComponent(this.actorId)}.tar"`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in the export process.
The method only handles the case where the account is not found. Consider adding more comprehensive error handling.
async exportData(c: Context) {
+ try {
const account = await this.loadAccount();
if (!account) return c.json({ error: "Actor not found" }, 404);
// Load and serialize posts
const postsData = await this.loadPosts();
// ... rest of the code ...
return c.body(exportTarballStream as Readable, 200, {
"Content-Type": "application/x-tar",
"Content-Disposition": `attachment; filename="account_export_${encodeURIComponent(this.actorId)}.tar"`,
});
+ } catch (error) {
+ console.error('Error during account export:', error);
+ return c.json({ error: "Failed to export account data" }, 500);
+ }
}📝 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.
| async exportData(c: Context) { | |
| const account = await this.loadAccount(); | |
| if (!account) return c.json({ error: "Actor not found" }, 404); | |
| // Load and serialize posts | |
| const postsData = await this.loadPosts(); | |
| const serializedPosts = postsData.map((post) => | |
| serializePost(post, { id: account.owner.id }, c.req.url), | |
| ); | |
| // Load and serialize lists | |
| const lists = await this.loadLists(); | |
| console.log("🚀 ~ AccountExporter ~ exportData ~ lists:", lists); | |
| const serializedLists = lists.map((list) => serializeList(list)); | |
| console.log( | |
| "🚀 ~ AccountExporter ~ exportData ~ serializedLists:", | |
| serializedLists, | |
| ); | |
| // Load and serialize followers | |
| const followers = await this.loadFollows("followers"); | |
| const serializedFollowers = this.serializeFollowers(followers); | |
| // Load and serialize following | |
| const followingAccounts = await this.loadFollows("following"); | |
| const serializedFollowing = this.serializeFollowing(followingAccounts); | |
| // Load and serialize bookmarks | |
| const bookmarks = await this.loadBookmarks(); | |
| const serializedBookmarks = this.serializeBookmarks(bookmarks); | |
| // Generate export tarball | |
| const exportTarballStream = exportActorProfile({ | |
| actorProfile: serializeAccount( | |
| { ...account, successor: null }, | |
| c.req.url, | |
| ), | |
| outbox: serializedPosts, | |
| lists: serializedLists, | |
| followers: serializedFollowers, | |
| followingAccounts: serializedFollowing, | |
| bookmarks: serializedBookmarks, | |
| }); | |
| return c.body(exportTarballStream as any, 200, { | |
| "Content-Type": "application/x-tar", | |
| "Content-Disposition": `attachment; filename="account_export_${encodeURIComponent(this.actorId)}.tar"`, | |
| }); | |
| } | |
| async exportData(c: Context) { | |
| try { | |
| const account = await this.loadAccount(); | |
| if (!account) return c.json({ error: "Actor not found" }, 404); | |
| // Load and serialize posts | |
| const postsData = await this.loadPosts(); | |
| const serializedPosts = postsData.map((post) => | |
| serializePost(post, { id: account.owner.id }, c.req.url), | |
| ); | |
| // Load and serialize lists | |
| const lists = await this.loadLists(); | |
| console.log("🚀 ~ AccountExporter ~ exportData ~ lists:", lists); | |
| const serializedLists = lists.map((list) => serializeList(list)); | |
| console.log( | |
| "🚀 ~ AccountExporter ~ exportData ~ serializedLists:", | |
| serializedLists, | |
| ); | |
| // Load and serialize followers | |
| const followers = await this.loadFollows("followers"); | |
| const serializedFollowers = this.serializeFollowers(followers); | |
| // Load and serialize following | |
| const followingAccounts = await this.loadFollows("following"); | |
| const serializedFollowing = this.serializeFollowing(followingAccounts); | |
| // Load and serialize bookmarks | |
| const bookmarks = await this.loadBookmarks(); | |
| const serializedBookmarks = this.serializeBookmarks(bookmarks); | |
| // Generate export tarball | |
| const exportTarballStream = exportActorProfile({ | |
| actorProfile: serializeAccount( | |
| { ...account, successor: null }, | |
| c.req.url, | |
| ), | |
| outbox: serializedPosts, | |
| lists: serializedLists, | |
| followers: serializedFollowers, | |
| followingAccounts: serializedFollowing, | |
| bookmarks: serializedBookmarks, | |
| }); | |
| return c.body(exportTarballStream as any, 200, { | |
| "Content-Type": "application/x-tar", | |
| "Content-Disposition": `attachment; filename="account_export_${encodeURIComponent(this.actorId)}.tar"`, | |
| }); | |
| } catch (error) { | |
| console.error('Error during account export:', error); | |
| return c.json({ error: "Failed to export account data" }, 500); | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 134-134: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
src/entities/exportAccount.ts
Outdated
| serializeBookmarks(bookmarks: schema.Bookmark[]) { | ||
| return { | ||
| "@context": "https://www.w3.org/ns/activitystreams", | ||
| id: "bookmarks.json", | ||
| type: "OrderedCollection", | ||
| orderedItems: bookmarks.map((bookmark) => bookmark.postId), | ||
| }; | ||
| } | ||
|
|
||
| serializeFollowing(followingAccounts: schema.Follow[]) { | ||
| return { | ||
| "@context": "https://www.w3.org/ns/activitystreams", | ||
| id: "following_accounts.json", | ||
| type: "OrderedCollection", | ||
| orderedItems: followingAccounts.map((account) => ({ | ||
| account: `${homeUrl}/accounts/${account.followingId}`, | ||
| showBoosts: account.shares, | ||
| notifyOnNewPosts: account.notify, | ||
| language: account.languages, | ||
| })), | ||
| }; | ||
| } | ||
|
|
||
| serializeFollowers(followers: schema.Follow[]) { | ||
| return { | ||
| "@context": "https://www.w3.org/ns/activitystreams", | ||
| id: "followers.json", | ||
| type: "OrderedCollection", | ||
| orderedItems: followers.map((follower) => ({ | ||
| account: `${homeUrl}/accounts/${follower.followerId}`, | ||
| followedSince: follower.created, | ||
| language: follower.languages, | ||
| })), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL validation and normalization for account references.
The serialization methods could be improved to handle URL construction more robustly.
Consider adding a helper method for URL construction:
+private normalizeUrl(path: string): string {
+ const base = homeUrl.endsWith('/') ? homeUrl : `${homeUrl}/`;
+ return new URL(path, base).toString();
+}
serializeFollowing(followingAccounts: schema.Follow[]) {
return {
"@context": "https://www.w3.org/ns/activitystreams",
id: "following_accounts.json",
type: "OrderedCollection",
orderedItems: followingAccounts.map((account) => ({
- account: `${homeUrl}/accounts/${account.followingId}`,
+ account: this.normalizeUrl(`accounts/${account.followingId}`),
showBoosts: account.shares,
notifyOnNewPosts: account.notify,
language: account.languages,
})),
};
}Apply similar changes to serializeFollowers method.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
src/entities/exportAccount.ts (4)
10-11: Add proper explanation for the biome-ignore commentThe
biome-ignorecomment lacks a meaningful explanation. Please provide a clear reason for ignoring the lint rule.-// biome-ignore lint/complexity/useLiteralKeys: <explanation> +// biome-ignore lint/complexity/useLiteralKeys: Using string literal for environment variable access
11-11: Enhance environment variable handlingConsider adding validation and type safety for the
HOME_URLenvironment variable.-const homeUrl = process.env["HOME_URL"] || "http://localhost:3000/"; +const homeUrl = (() => { + const url = process.env["HOME_URL"] || "http://localhost:3000/"; + try { + return new URL(url).toString(); + } catch { + console.warn(`Invalid HOME_URL: ${url}, falling back to default`); + return "http://localhost:3000/"; + } +})();
28-33: Add pagination for posts queryLoading all posts at once could cause performance issues for accounts with many posts. Consider implementing pagination.
- async loadPosts() { + async loadPosts(limit = 100, offset = 0) { return db.query.posts.findMany({ where: eq(schema.posts.accountId, this.actorId), + limit, + offset, with: getPostRelations(this.actorId), }); }
108-108: Remove debug console.log statementsRemove console.log statements used for debugging before merging to production.
- console.log("🚀 ~ AccountExporter ~ exportData ~ lists:", lists); const serializedLists = lists.map((list) => serializeList(list)); - console.log( - "🚀 ~ AccountExporter ~ exportData ~ serializedLists:", - serializedLists, - );Also applies to: 110-113
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (1)
src/entities/exportAccount.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/entities/exportAccount.ts
[error] 140-140: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🔇 Additional comments (2)
src/entities/exportAccount.ts (2)
140-140: Replace any type with proper typing
Using any disables type checking. The type of exportTarballStream should be properly defined.
🧰 Tools
🪛 Biome (1.9.4)
[error] 140-140: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
144-147: 🛠️ Refactor suggestion
Enhance error handling with specific error types
The catch block should handle different types of errors appropriately.
- } catch (e) {
- console.error(e);
- return c.json({ error: "Internal server error occurred" }, 500);
+ } catch (e) {
+ console.error('Account export error:', e);
+ if (e instanceof TypeError || e instanceof RangeError) {
+ return c.json({ error: "Invalid data format" }, 400);
+ }
+ if (e instanceof Error && e.message.includes('database')) {
+ return c.json({ error: "Database operation failed" }, 503);
+ }
+ return c.json({ error: "Internal server error occurred" }, 500);
}Likely invalid or redundant comment.
src/entities/exportAccount.ts
Outdated
| orderedItems: followingAccounts.map((account) => ({ | ||
| account: this.normalizeUrl(`accounts/${account.followingId}`), | ||
| showBoosts: account.shares, | ||
| notifyOnNewPosts: account.notify, | ||
| language: account.languages, | ||
| })), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety to serialization methods
The mapping functions should handle potential null or undefined values in the data.
orderedItems: followingAccounts.map((account) => ({
account: this.normalizeUrl(`accounts/${account.followingId}`),
showBoosts: account.shares,
notifyOnNewPosts: account.notify,
- language: account.languages,
+ language: account.languages ?? null,
})),📝 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.
| orderedItems: followingAccounts.map((account) => ({ | |
| account: this.normalizeUrl(`accounts/${account.followingId}`), | |
| showBoosts: account.shares, | |
| notifyOnNewPosts: account.notify, | |
| language: account.languages, | |
| })), | |
| orderedItems: followingAccounts.map((account) => ({ | |
| account: this.normalizeUrl(`accounts/${account.followingId}`), | |
| showBoosts: account.shares, | |
| notifyOnNewPosts: account.notify, | |
| language: account.languages ?? null, | |
| })), |
src/entities/exportAccount.ts
Outdated
| constructor(actorId: string) { | ||
| this.actorId = actorId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for actorId
The constructor should validate the actorId to ensure it's not empty or malformed.
constructor(actorId: string) {
+ if (!actorId?.trim()) {
+ throw new Error('actorId cannot be empty');
+ }
this.actorId = actorId;
}📝 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.
| constructor(actorId: string) { | |
| this.actorId = actorId; | |
| } | |
| constructor(actorId: string) { | |
| if (!actorId?.trim()) { | |
| throw new Error('actorId cannot be empty'); | |
| } | |
| this.actorId = actorId; | |
| } |
[ci skip]
[ci skip]
[ci skip]
# Conflicts: # bun.lockb
There was a problem hiding this comment.
Actionable comments posted: 35
🧹 Outside diff range and nitpick comments (32)
src/index.tsx (1)
36-43: Limit debug logging in production environmentsLogging paths for not found assets can expose sensitive information. It's advisable to limit debug logging to development environments.
Consider modifying the code to log debug information only when in a development environment:
+const isDevelopment = process.env["NODE_ENV"] === "development"; app.use( "/public/*", serveStatic({ root: join(import.meta.dirname, "public"), rewriteRequestPath: (path) => path.substring("/public".length), - onNotFound: (path) => console.debug({ path }), + onNotFound: (path) => { + if (isDevelopment) { + console.debug({ path }); + } + }, }), );src/pages/federation.tsx (2)
57-63: Implement input validation for the 'uri' fieldTo ensure the correctness and security of the input, consider adding validation for the
urifield to verify that it is a valid fediverse handle or URI.You can use a validation library or regular expressions to validate the input before processing it.
42-48: Refactor nested ternary operators for better readabilityThe nested ternary operator used here can be hard to read and maintain.
Consider refactoring using an
ifstatement or a function:- {done === "refresh:account" ? ( - <p>Account has been refreshed.</p> - ) : done === "refresh:post" ? ( - <p>Post has been refreshed.</p> - ) : ( - <p>Use this when you see outdated remote account/post data.</p> - )} + {(() => { + if (done === "refresh:account") { + return <p>Account has been refreshed.</p>; + } else if (done === "refresh:post") { + return <p>Post has been refreshed.</p>; + } else { + return <p>Use this when you see outdated remote account/post data.</p>; + } + })()}src/pages/emojis.tsx (1)
173-176: Validate image MIME types on the server sideRelying solely on client-side
acceptattribute can be bypassed. Ensure the server validates the image MIME type to be one of the accepted types.Add server-side validation for MIME types:
const extension = mime.getExtension(image.type); + const allowedTypes = ["image/png", "image/jpeg", "image/gif", "image/webp"]; + if (!allowedTypes.includes(image.type)) { + return c.text("Unsupported image type", 400); + } if (!extension) { return c.text("Unsupported image type", 400); }src/api/v1/notifications.ts (1)
Line range hint
105-168: Refactor duplicated filtering logic to improve maintainabilityThe filtering conditions to exclude notifications from muted and blocked accounts are repeated across different notification types. Consider abstracting this logic into a shared function or query builder helper to reduce code duplication and enhance readability.
src/api/v1/timelines.ts (1)
Line range hint
86-166: Refactor repeated filtering logic to improve maintainabilityThe conditions to exclude posts from muted and blocked accounts are repeated across multiple endpoints. Consider abstracting this filtering logic into a shared function or query helper to reduce code duplication and enhance readability.
src/federation/inbox.ts (2)
Line range hint
775-804: Ensure consistency when handling account movesIn the
onAccountMovedfunction, after migrating followers to the new account, ensure that the old account's state reflects the move to prevent synchronization issues. Consider updating or flagging the old account to indicate it has moved.
Line range hint
624-642: Validate emoji reactions to prevent invalid entriesIn the
onEmojiReactionAddedfunction, ensure that the emoji content is properly validated to prevent injection of invalid or malicious data. Implement checks to confirm that the emoji format is acceptable and sanitize the input as necessary.src/schema.ts (2)
511-511: Clarify the necessity of 'pollIdIndexIdx' indexAn index
pollIdIndexIdxis created on thepollIdandindexcolumns in thepoll_optionstable. If a primary key or unique constraint already exists on these columns, this index might be redundant. Evaluate whether this additional index is required for query performance or if it can be omitted.
687-688: Ensure consistency in index naming conventionsThe indices
postIdIdxandpostIdAccountIdIdxin thereactionstable are added for query optimization. Consider using a consistent naming convention for indexes throughout the schema to improve maintainability and clarity.src/api/v1/statuses.ts (2)
119-128: Improve idempotency key handling and responseWhen checking for existing posts using the idempotency key, consider:
- Handling cases where multiple posts may match the key.
- Ensuring the time window of one hour is appropriate for your application.
- Providing a clear response when a duplicate post is detected.
221-221: Validate media IDs before proceeding with post creationCurrently, if media IDs are invalid or already associated with another post, the transaction is rolled back. Consider validating all media IDs upfront before starting the transaction to provide immediate feedback to the user.
.env.sample (1)
15-17: Add validation requirements for storage configurationThe new storage-related environment variables lack documentation about their required format and validation rules.
Add comments explaining:
-DRIVE_DISK= +DRIVE_DISK= # Required. Valid values: 'fs' or 's3' -ASSET_URL_BASE= +ASSET_URL_BASE= # Required when DRIVE_DISK is set. Must be a valid URL with trailing slash -FS_ASSET_PATH= +FS_ASSET_PATH= # Required when DRIVE_DISK='fs'. Must be an absolute path with write permissionssrc/sentry.ts (2)
13-16: Consider configurable sampling rate for productionSetting
tracesSampleRate: 1.0captures all transactions, which might impact performance and costs in production.Consider making the sampling rate configurable:
const client = init({ dsn, - tracesSampleRate: 1.0, + tracesSampleRate: process.env.SENTRY_SAMPLE_RATE + ? parseFloat(process.env.SENTRY_SAMPLE_RATE) + : 1.0, });
17-23: Enhance error handling for initialization failureThe error logging could be more descriptive to aid in troubleshooting.
if (client == null) { - logger.error("Failed to initialize Sentry."); + logger.error("Failed to initialize Sentry client. Please verify DSN and permissions."); return; } getGlobalScope().setClient(client); setCurrentClient(client); - logger.debug("Sentry initialized."); + logger.debug(`Sentry initialized successfully with DSN: ${dsn.replace(/[:@]/g, '*')}`);src/components/NewAccountPage.tsx (1)
13-13: Document the purpose of thenewsfieldThe purpose and implications of the
newsboolean field are not clear from the code. Consider adding JSDoc comments to explain its usage and impact on account creation.drizzle/0053_instances.sql (2)
18-22: Consider cascade behavior for foreign keyThe current foreign key constraint uses
ON DELETE no action ON UPDATE no action, which might cause issues when managing instance data. Consider whetherCASCADEorSET NULLwould be more appropriate for your use case.
1-6: Add index for foreign key columnConsider adding an index on the
instance_hostcolumn in the accounts table to improve query performance.CREATE TABLE IF NOT EXISTS "instances" ( "host" text PRIMARY KEY NOT NULL, "software" text, "software_version" text, "created" timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL ); +CREATE INDEX IF NOT EXISTS "idx_accounts_instance_host" ON "accounts" ("instance_host");src/logging.ts (1)
49-68: Consider log level configuration for account export functionalityThe logger configurations look good, but consider:
- Adding specific logging category for account export operations
- Ensuring appropriate log levels for sensitive operations
- Monitoring export performance through logs
Add a new logger configuration for account export:
{ category: ["logtape", "meta"], lowestLevel: "warning", sinks: ["console", "sentry"], }, + { + category: "account-export", + lowestLevel: LOG_LEVEL, + sinks: ["console", "sentry"], + },src/components/Profile.tsx (2)
17-17: Consider keeping fragment instead of divUnless there's a specific need for CSS targeting or layout purposes, using a fragment (
<>) would be more efficient than adding an extra div wrapper.Also applies to: 77-77
19-23: Move inline styles to CSS classesConsider moving the inline styles to a CSS module or styled component for better maintainability and reusability.
- style="margin-bottom: 1em; width: 100%;" + className="profile-cover-image"Add to your CSS:
.profile-cover-image { margin-bottom: 1em; width: 100%; }docs/src/content/docs/zh-cn/install/env.mdx (2)
30-38: Security warning looks good but could be more prominentThe security implications of
BEHIND_PROXYare well explained, but consider adding a warning emoji (⚠️ ) to make the security note more visible.
127-136: Comprehensive S3 permissions documentationThe S3 permissions list is complete and well-organized. Consider adding examples of IAM policy JSON for easier implementation.
docs/src/content/docs/ja/install/env.mdx (2)
108-109: Add timeline for default value deprecationConsider adding when the default value will be removed to help users prepare for the change.
-将来的にはデフォルトは無くなる予定なので、明示的に設定する事をお勧めします。 +将来的には（バージョン2.0.0で）デフォルトは無くなる予定なので、明示的に設定する事をお勧めします。
186-191: Enhance security guidance for AWS credentialsConsider adding a security note about protecting these credentials and using environment-specific keys.
#### `AWS_ACCESS_KEY_ID` <Badge text="S3ドライバで必須" variant="caution" /> S3互換オブジェクトストレージのアクセスキー。 + +<Aside type="caution"> + 本番環境と開発環境で異なるアクセスキーを使用することを強く推奨します。 + また、必要最小限の権限のみを持つIAMユーザーを作成してください。 +</Aside>docs/src/content/docs/ko/install/env.mdx (2)
108-109: Add timeline for default value deprecationConsider adding when the default value will be removed to help users prepare for the change.
-하위 호환성을 위해 `s3`가 기본값이지만, 앞으로는 기본값이 사라질 예정이므로 명시적으로 설정하는 것이 좋습니다. +하위 호환성을 위해 `s3`가 기본값이지만, 앞으로는 (버전 2.0.0에서) 기본값이 사라질 예정이므로 명시적으로 설정하는 것이 좋습니다.
183-188: Enhance security guidance for AWS credentialsConsider adding a security note about protecting these credentials and using environment-specific keys.
#### `AWS_ACCESS_KEY_ID` <Badge text="S3 드라이버에서 필수" variant="caution" /> S3 호환 오브젝트 스토리지 액세스 키. + +<Aside type="caution"> + 프로덕션 환경과 개발 환경에서 서로 다른 액세스 키를 사용할 것을 강력히 권장합니다. + 또한, 필요한 최소한의 권한만 가진 IAM 사용자를 만드세요. +</Aside>src/components/AccountForm.tsx (2)
5-5: Consider using enum for method typeThe method type could be more TypeScript idiomatic by using an enum or const assertion.
- method?: "get" | "post" | "dialog"; + method?: typeof FORM_METHODS[number]; + +const FORM_METHODS = ["get", "post", "dialog"] as const;
137-148: Improve accessibility and semantics of the news subscription sectionThe news subscription checkbox could benefit from improved accessibility and semantic markup.
<fieldset> + <legend className="sr-only">Notifications Preferences</legend> <label> <input type="checkbox" name="news" value="true" checked={props.values?.news} + aria-describedby="news-description" />{" "} Receive news and updates of Hollo by following the official Hollo - account (<tt>{props.officialAccount}</tt>) + account <code className="inline-code">{props.officialAccount}</code> </label> + <small id="news-description"> + You can unfollow the official account at any time to stop receiving updates. + </small> </fieldset>CHANGES.md (1)
7-14: Consider consolidating duplicate changelog entries.The bug fixes for
Undo(Like),EmojiReact, andUndo(EmojiReact)activities onQuestionobjects are duplicated in versions 0.3.1 and 0.2.4. Consider consolidating these entries or clarifying why they appear in both versions.Also applies to: 45-52
src/pages/profile/index.tsx (1)
28-28: LGTM! Consider making WINDOW size configurableThe constant
WINDOW = 30is a reasonable default for social media content pagination.Consider making this configurable through environment variables:
-const WINDOW = 30; +const WINDOW = Number(process.env.POSTS_PER_PAGE) || 30;drizzle/meta/0053_snapshot.json (1)
1119-1223: Consider adding indexes for media queriesThe media table would benefit from additional indexes for common query patterns.
Consider adding the following indexes:
- Composite index on (type, created) for media type filtering
- Index on url for duplicate detection
- Index on thumbnail_url for thumbnail lookups
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockbis excluded by!**/bun.lockbsrc/public/favicon.pngis excluded by!**/*.png
📒 Files selected for processing (67)
.devcontainer/devcontainer.json(1 hunks).env.sample(1 hunks).github/FUNDING.yaml(1 hunks).gitignore(1 hunks)CHANGES.md(1 hunks)Dockerfile(1 hunks)SECURITY.md(1 hunks)biome.json(1 hunks)compose-fs.yaml(1 hunks)compose.yaml(2 hunks)docs/src/content/docs/install/env.mdx(3 hunks)docs/src/content/docs/install/manual.mdx(0 hunks)docs/src/content/docs/ja/install/env.mdx(3 hunks)docs/src/content/docs/ja/install/manual.mdx(0 hunks)docs/src/content/docs/ko/install/env.mdx(3 hunks)docs/src/content/docs/ko/install/manual.mdx(0 hunks)docs/src/content/docs/zh-cn/install/env.mdx(3 hunks)docs/src/content/docs/zh-cn/install/manual.mdx(0 hunks)drizzle/0053_instances.sql(1 hunks)drizzle/0054_indices.sql(1 hunks)drizzle/0055_indices.sql(1 hunks)drizzle/0056_posts.idempotence_key.sql(1 hunks)drizzle/meta/0053_snapshot.json(1 hunks)drizzle/meta/0054_snapshot.json(1 hunks)drizzle/meta/0055_snapshot.json(1 hunks)drizzle/meta/0056_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)mise.toml(1 hunks)package.json(3 hunks)src/api/v1/accounts.ts(14 hunks)src/api/v1/media.ts(3 hunks)src/api/v1/notifications.ts(1 hunks)src/api/v1/statuses.ts(5 hunks)src/api/v1/timelines.ts(8 hunks)src/api/v2/index.ts(5 hunks)src/components/AccountForm.tsx(3 hunks)src/components/DashboardLayout.tsx(2 hunks)src/components/Layout.tsx(1 hunks)src/components/LoginForm.tsx(1 hunks)src/components/NewAccountPage.tsx(2 hunks)src/components/OtpForm.tsx(1 hunks)src/components/Profile.tsx(2 hunks)src/components/SetupForm.tsx(1 hunks)src/entities/exportAccount.ts(1 hunks)src/entities/status.ts(1 hunks)src/federation/account.ts(12 hunks)src/federation/emoji.ts(1 hunks)src/federation/inbox.ts(13 hunks)src/federation/index.ts(7 hunks)src/federation/post.ts(14 hunks)src/index.tsx(2 hunks)src/logging.ts(2 hunks)src/media.ts(3 hunks)src/pages/accounts.tsx(18 hunks)src/pages/data.tsx(0 hunks)src/pages/emojis.tsx(4 hunks)src/pages/federation.tsx(1 hunks)src/pages/index.tsx(2 hunks)src/pages/profile/index.tsx(7 hunks)src/public/pico.colors.min.css(1 hunks)src/public/pico.min.css(1 hunks)src/redis.ts(0 hunks)src/s3.ts(0 hunks)src/schema.ts(12 hunks)src/sentry.ts(1 hunks)src/storage.ts(1 hunks)src/text.ts(4 hunks)
💤 Files with no reviewable changes (7)
- src/redis.ts
- docs/src/content/docs/install/manual.mdx
- src/pages/data.tsx
- docs/src/content/docs/zh-cn/install/manual.mdx
- docs/src/content/docs/ko/install/manual.mdx
- docs/src/content/docs/ja/install/manual.mdx
- src/s3.ts
✅ Files skipped from review due to trivial changes (9)
- .github/FUNDING.yaml
- drizzle/0056_posts.idempotence_key.sql
- mise.toml
- .gitignore
- SECURITY.md
- Dockerfile
- src/public/pico.colors.min.css
- drizzle/meta/_journal.json
- drizzle/0054_indices.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- src/api/v2/index.ts
- package.json
🧰 Additional context used
🪛 LanguageTool
CHANGES.md
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...S_ASSET_PATHenvironment variable. - AddedASSET_URL_BASE` environment variable t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d SENTRY_DSN environment variable. - Added pagination to the profile page. [[#40]]...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[duplication] ~145-~145: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~148-~148: Consider using a different verb for a more formal wording.
Context: ... from the administration dashboard to fix the issue. - Fixed the federated (publ...
(FIX_RESOLVE)
[uncategorized] ~184-~184: Possible missing comma found.
Context: ...ixed the last page in the profile using Moshidon leading to infinite pagination. [[#48...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/src/content/docs/install/env.mdx
[uncategorized] ~107-~107: Possible missing preposition found.
Context: ...d s3 (S3-compatible object storage). Defaults to s3 for backward compatibility, but...
(AI_HYDRA_LEO_MISSING_IT)
[style] ~108-~108: Style-wise, it’s not ideal to insert an adverb (‘explicitly’) in the middle of an infinitive construction (‘to configure’). Try moving the adverb to avoid split infinitives.
Context: ...` for backward compatibility, but it is recommended to explicitly configure the driver as the default value will be removed in...
(SPLIT_INFINITIVE)
🪛 Gitleaks (8.21.2)
.devcontainer/devcontainer.json
15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Biome (1.9.4)
src/entities/exportAccount.ts
[error] 77-77: This property value named account is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named account
(lint/suspicious/noDuplicateObjectKeys)
[error] 40-40: Shouldn't redeclare 'column'. Consider to delete it or rename it.
'column' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (45)
src/components/DashboardLayout.tsx (2)
54-59: Verify the federation route implementation.
The menu item update looks good and maintains consistency with other menu items. However, we should verify that the /federation route exists and is properly implemented.
✅ Verification successful
Federation route is properly implemented and registered
The federation route is correctly implemented in src/pages/federation.tsx and properly registered in the application router at src/pages/index.tsx. The component includes a complete federation management interface with features for refreshing remote accounts/posts and monitoring federation queue messages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the federation route handler
# Search for route definitions
rg --type typescript '/federation'
# Search for specific route handler using AST
ast-grep --pattern 'app.$_("/federation", $_)'
Length of output: 118
Script:
#!/bin/bash
# Search for federation route definitions with ripgrep
rg -t ts -t tsx -t js -t jsx "(/federation|path.*federation)" -A 3
# Find files that might contain federation route
fd -e ts -e tsx -e js -e jsx | grep -i "route\|page\|federation"
# Search for any federation-related components
rg -t ts -t tsx -t js -t jsx "Federation" -A 3
Length of output: 711
Script:
#!/bin/bash
# Check the content of federation page component
cat src/pages/federation.tsx
# Look for federation route registration in any router setup files
rg -g '*.{ts,tsx,js,jsx}' "federation.*route|route.*federation"
# Check imports and usage of federation page
rg -g '*.{ts,tsx,js,jsx}' "from.*federation|import.*federation"
Length of output: 6622
4-4: Verify type usage across the codebase.
The Menu type change from "data" to "federation" looks good, but as this is a breaking change, we should verify all usages.
✅ Verification successful
Type change from "data" to "federation" is safe to proceed
The search results show that:
- The
Menutype is only defined and used insrc/components/DashboardLayout.tsx - It's only used once as a prop type (
selectedMenu?: Menu) - There are no other files importing or using the Menu type
- The old "data" string literal is not used in any TypeScript files in relation to menu options
- A corresponding
federation.tsxpage exists atsrc/pages/federation.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all files using the Menu type or referencing "data" as a menu option
# to ensure the change is applied consistently.
# Search for Menu type usage
ast-grep --pattern 'Menu = $_'
# Search for string literals that might be using the old "data" menu option
rg '"data"' --type typescript
Length of output: 569
Script:
#!/bin/bash
# Let's try a different approach to find Menu type usage and "data" references
# Search for Menu type usage in TypeScript/TSX files
rg "type Menu =" -t ts -t tsx
# Search for files importing Menu type
rg "import.*Menu.*from" -t ts -t tsx
# Search for "data" as a string literal in TypeScript/TSX files
rg '"data"' -t ts -t tsx
# Search for any usage of Menu type as a prop or variable
rg ": Menu" -t ts -t tsx
Length of output: 236
Script:
#!/bin/bash
# Let's try without explicit file type flags since rg seems to not recognize ts/tsx
# Find all TypeScript/TSX files first
fd "\.(ts|tsx)$" | while read -r file; do
echo "=== $file ==="
# Search for Menu type definition and usage
rg "type Menu =|import.*Menu|: Menu|\"data\"" "$file" || true
done
Length of output: 11710
src/public/pico.min.css (1)
1-3: Ensure compliance with Pico CSS's MIT License
Since Pico CSS is distributed under the MIT License, please ensure that you're complying with its terms. This typically involves including the license text in your project.
Run the following script to check if the MIT License is included in your project:
src/index.tsx (1)
64-67: Verify compatibility of the updated default export
Changing the default export to include both fetch and port may affect how the application is imported and used elsewhere in the codebase. Ensure that all imports and usage of the default export are updated accordingly.
Run the following script to find all imports of src/index.tsx and check for compatibility:
src/media.ts (1)
21-21:
Ensure all calls to uploadThumbnail include the new url parameter
The uploadThumbnail function signature now includes an additional url parameter. Verify that all calls to this function throughout the codebase are updated to pass the new parameter.
Run the following script to find all usages of uploadThumbnail:
✅ Verification successful
Let me verify the function definition to ensure we have the complete context.
Let me try a different pattern to get the function definition.
Based on the search results from both commands, I can now generate the final response:
All calls to uploadThumbnail are properly updated with the url parameter
The codebase search shows that all existing calls to uploadThumbnail include the required url parameter:
src/api/v1/media.ts: passesc.req.urlsrc/federation/post.ts: passesbaseUrl
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all calls to `uploadThumbnail` and check if they include the `url` parameter.
rg 'uploadThumbnail\s*\(' -A 2
Length of output: 453
Script:
#!/bin/bash
# Get the full function definition of uploadThumbnail
ast-grep --pattern 'export async function uploadThumbnail($_) { $$$ }'
Length of output: 72
Script:
#!/bin/bash
# Get the full function definition and surrounding context
rg -B2 -A10 "export async function uploadThumbnail" src/media.ts
Length of output: 408
src/pages/federation.tsx (1)
135-136: Verify that retrieving the first account owner is intentional
The code retrieves the first account owner without any filtering criteria:
const owner = await db.query.accountOwners.findFirst({});Ensure that this is intentional and that it does not introduce security risks or unintended behavior, especially if multiple account owners exist.
Run the following script to check the number of account owners:
✅ Verification successful
From analyzing the codebase, I can now provide a final response regarding the review comment about retrieving the first account owner without filtering criteria.
Using findFirst() without filtering in federation.tsx is intentional and safe
The unfiltered findFirst() query in federation.tsx is intentionally used to retrieve any account owner for federation-related functionality. This is evidenced by:
- The schema shows
accountOwnerstable has a 1:1 relationship withaccountstable, where each account can have at most one owner. - The same pattern of retrieving any account owner is used in the instance API endpoints (
src/api/v1/instance.tsandsrc/api/v2/instance.ts). - The context of the code in
federation.tsxshows it's used for federation document loading, where any valid account owner can be used to initialize the federation context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Count the number of account owners in the database.
# Test: Expect only one account owner. If more, verification is needed.
psql $DATABASE_URL -c "SELECT COUNT(*) FROM account_owners;"
Length of output: 143
Script:
#!/bin/bash
# Let's analyze the codebase to understand the context and usage of accountOwners
# Check schema definition for accountOwners
fd "schema.ts" --exec cat {}
# Find usages of accountOwners.findFirst
rg "accountOwners.findFirst" -A 3
# Look for any tests or documentation related to account owners
rg -i "account.?owner" --type md
Length of output: 37758
src/federation/account.ts (1)
120-130: Verify 'actor.id.host' is always available
The code uses actor.id.host, assuming it is always defined. If actor.id.host is undefined, it could cause errors.
Add a check to ensure actor.id.host is valid:
+ if (!actor.id.host) {
+ // Handle the error appropriately
+ throw new Error("Actor ID host is undefined");
+ }
await db
.insert(schema.instances)
.values({
host: actor.id.host,
...instanceValues,
})Alternatively, ensure that actor.id.host cannot be undefined based on previous validations.
src/api/v1/timelines.ts (1)
Line range hint 86-166: Ensure database indexes support new filtering conditions
The added filters increase the complexity of the queries. Verify that appropriate indexes exist on the mutes and blocks tables to optimize query performance and prevent potential slowdowns.
Run the following script to list the indexes on the relevant tables:
✅ Verification successful
Existing indexes are sufficient for the filtering conditions
The database schema already has appropriate indexes to support the filtering conditions:
- The
mutestable has a unique constraint on(account_id, muted_account_id), which creates an index that will be used for filtering muted accounts - The
blockstable has a primary key on(account_id, blocked_account_id), which creates an index that will be used for filtering blocked accounts
These composite indexes will efficiently support the WHERE clauses in the queries that filter posts from muted and blocked accounts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: List indexes on the 'mutes' and 'blocks' tables.
echo "Indexes on 'mutes' table:"
psql -c "\d+ mutes"
echo "Indexes on 'blocks' table:"
psql -c "\d+ blocks"
Length of output: 370
Script:
#!/bin/bash
# Search for SQL schema files or migration files that might contain index definitions
fd -e sql -e prisma
# If found, let's examine their content
fd -e sql -e prisma --exec cat {}
Length of output: 35049
Script:
#!/bin/bash
# Search for indexes on mutes and blocks tables in migration files
echo "Searching for indexes on mutes table:"
rg -A 2 "CREATE INDEX.*mutes" drizzle/
echo "Searching for indexes on blocks table:"
rg -A 2 "CREATE INDEX.*blocks" drizzle/
# Search for unique constraints that might act as indexes
echo "Searching for unique constraints on mutes table:"
rg -A 2 "UNIQUE.*mutes" drizzle/
echo "Searching for unique constraints on blocks table:"
rg -A 2 "UNIQUE.*blocks" drizzle/
Length of output: 541
Script:
#!/bin/bash
# Search for table creation statements and constraints for mutes and blocks
echo "Examining mutes table creation:"
rg -A 10 "CREATE TABLE.*mutes" drizzle/
echo "Examining blocks table creation:"
rg -A 10 "CREATE TABLE.*blocks" drizzle/
# Search for primary key constraints which can act as indexes
echo "Searching for primary key constraints:"
rg "PRIMARY KEY.*mutes|PRIMARY KEY.*blocks" drizzle/
Length of output: 1676
src/federation/post.ts (3)
94-96: Verify 'baseUrl' parameter is correctly integrated into 'persistPost'
The persistPost function now includes a baseUrl parameter. Ensure that all calls to persistPost throughout the codebase have been updated to pass the baseUrl argument, and that the parameter is effectively used within the function to handle URL management consistently.
Run the following script to find calls to persistPost that may be missing the baseUrl parameter:
✅ Verification successful
All calls to persistPost are correctly using the baseUrl parameter
Based on the comprehensive search results, all calls to persistPost throughout the codebase are properly passing the required baseUrl parameter. The function is consistently called with the correct number of arguments in all locations:
- In API endpoints (
src/api/v1/statuses.ts,src/api/v2/index.ts): usingc.req.urlas baseUrl - In federation handlers (
src/federation/inbox.ts): usingctx.originas baseUrl - In account-related operations (
src/federation/account.ts): passing through thebaseUrlparameter - In post-related operations (
src/federation/post.ts): properly forwarding thebaseUrlparameter - In page handlers (
src/pages/federation.tsx,src/pages/accounts.tsx): usingc.req.urlas baseUrl
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find calls to 'persistPost' missing 'baseUrl' parameter.
# Search for calls to 'persistPost' that have fewer than 3 arguments
ast-grep --pattern $'persistPost($_, $_)' --lang ts
# Expected: No matches. If matches are found, these calls may be missing 'baseUrl'.
Length of output: 52
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# First, let's find all calls to persistPost with any number of arguments
ast-grep --pattern 'persistPost($$$)' --lang ts
# Also search for the function definition to understand its parameters
ast-grep --pattern 'function persistPost($$$)' --lang ts
# Additionally, let's use ripgrep to find any references to persistPost
rg "persistPost" -A 2
Length of output: 6397
359-364: Confirm 'persistAccountByIri' function accepts 'baseUrl' parameter
At lines 359-364, persistAccountByIri is invoked with baseUrl as an argument. Verify that the function's signature has been updated to accept baseUrl and that it utilizes this parameter correctly for URL handling.
Run the following script to check the function definition:
✅ Verification successful
Let me gather more information about the function parameters and implementation.
Let me try a different approach to get the function definition.
Function signature correctly includes baseUrl parameter
The persistAccountByIri function's signature in src/federation/account.ts correctly includes the baseUrl parameter of type URL | string as its third argument, matching the usage shown in the code under review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Confirm 'persistAccountByIri' accepts 'baseUrl'.
# Search for the definition of 'persistAccountByIri'
rg 'function persistAccountByIri\(' -A 3 -t ts
# Expected: The function signature includes 'baseUrl' as a parameter.
Length of output: 262
Script:
#!/bin/bash
# Get the full function definition and implementation of persistAccountByIri
ast-grep --pattern 'export async function persistAccountByIri($_) {
$$$
}'
Length of output: 78
Script:
#!/bin/bash
# Get more context around the function definition
rg 'export async function persistAccountByIri' -B 2 -A 15 src/federation/account.ts
Length of output: 522
76-83: Ensure 'ChatMessage' is fully integrated throughout post handling
The addition of ChatMessage to the ASPost type and the isPost function broadens the types of posts handled by the system. Verify that all functions and logical branches that process ASPost instances have been updated to accommodate ChatMessage objects appropriately.
Run the following script to identify usages of ASPost and check for handling of ChatMessage:
✅ Verification successful
Let me gather more information about how posts are handled in the codebase.
Let me check one more aspect to see how posts are processed, particularly focusing on any switch statements or type-specific handling.
Based on the gathered information, I can now provide the final response:
ChatMessage integration appears to be properly handled
The codebase shows that ChatMessage is correctly integrated in the core type system and type checking:
ASPosttype union includesChatMessageisPosttype guard properly checks forChatMessageinstances- The only usage of
ASPostoutside of post.ts is in text.ts for quote handling, which usesisPost()for type checking and doesn't require type-specific processing
The implementation treats ChatMessage uniformly with other post types, and there's no evidence of type-specific handling that would need updates for the new type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of ASPost and check for ChatMessage handling.
# Find files where ASPost is used
rg 'ASPost' -t ts
# Check for logic that handles ChatMessage instances
rg 'instanceof ChatMessage' -t ts
Length of output: 579
Script:
#!/bin/bash
# Find all switch/case or if/else blocks that handle different post types
ast-grep --pattern 'if ($_ instanceof Article) {
$$$
}'
# Also check for any functions that specifically handle post types
rg -A 3 'function.*\(.*:.*(?:Article|Note|Question|ChatMessage).*\)' -t ts
# Look for any post-related type handling
rg 'type.*=.*(?:Article|Note|Question|ChatMessage)' -t ts
# Check the text.ts file content since it uses ASPost
cat src/text.ts
Length of output: 9557
Script:
#!/bin/bash
# Look for any post type-specific processing
ast-grep --pattern 'switch ($_.type) {
$$$
}'
# Check for any other files that might process posts
rg -A 5 'function.*process.*Post' -t ts
# Look for any other type-specific handling
rg -B 2 -A 2 'case "(Article|Note|Question)"' -t ts
Length of output: 145
src/schema.ts (1)
175-182: Review the definition of the new 'instances' table
The instances table has been introduced to store information about different instances. Confirm that:
- The
hostfield is correctly set as the primary key. - Optional fields like
softwareandsoftwareVersionare adequately defined. - The
createdtimestamp is set up correctly with default values. - Necessary indexes are in place for efficient querying.
src/api/v1/accounts.ts (1)
548-560: Ensure proper fetching of remote actor posts
The code fetches remote actor posts only if the post count is less than REMOTE_ACTOR_FETCH_POSTS. Verify that the persistAccountPosts function correctly utilizes c.req.url and handles any exceptions that may occur during the fetch process.
Run the following script to check the usage of persistAccountPosts:
✅ Verification successful
Let me check the implementation of persistAccountPosts to verify error handling.
Let me try a different approach to find the implementation using ripgrep.
Let me check the error handling in the persistPost function since it's called within persistAccountPosts.
Based on the code analysis, I can now provide the final response:
Remote actor post fetching implementation is properly handled
The persistAccountPosts function correctly:
- Uses
c.req.urlas thebaseUrlparameter consistently across all calls - Implements error handling through:
- Early returns for invalid conditions (
fetchPosts < 1, invalid actor) suppressErroroption being passed through the options object- Proper null checks on outbox collection and post objects
- Early returns for invalid conditions (
- Properly limits fetching through the
REMOTE_ACTOR_FETCH_POSTSparameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find calls to 'persistAccountPosts' and verify parameters.
rg 'persistAccountPosts\(' -A 5 -t ts
# Expected: Confirm that 'c.req.url' is being passed as 'baseUrl', and error handling is in place.
Length of output: 1194
Script:
#!/bin/bash
# Find the implementation of persistAccountPosts function
ast-grep --pattern 'export async function persistAccountPosts($_) {
$$$
}'
Length of output: 78
Script:
#!/bin/bash
# Find the implementation of persistAccountPosts function with more context
rg -t ts "export async function persistAccountPosts" -A 30 src/federation/account.ts
Length of output: 1093
Script:
#!/bin/bash
# Find the implementation of persistPost function
rg -t ts "export async function persistPost" -A 20 src/federation/post.ts
Length of output: 658
src/api/v1/statuses.ts (1)
285-287: Ensure 'documentLoader' uses correct parameters
When obtaining the documentLoader, confirm that passing { username: owner.handle } resolves correctly in all contexts and that it aligns with the expectations of the getDocumentLoader function.
drizzle/0055_indices.sql (1)
1-4: Indexes Added Successfully
The index creation statements are correct and will help optimize database queries.
src/pages/index.tsx (1)
6-6: LGTM with verification needed.
The addition of the federation route and its import look correct. However, let's verify that this is the intended location for the account export functionality.
Also applies to: 23-23
biome.json (1)
12-13: Clarify the relationship to PR objectives.
The addition of "src/public/" to the ignore list seems unrelated to the Account Export API implementation. Consider moving this change to a separate PR focused on tooling configuration.
✅ Verification successful
Ignore list addition is justified for static assets
The src/public directory contains static assets (CSS files and favicon) that don't need linting/formatting. Adding this directory to Biome's ignore list is a reasonable maintenance change, even if not directly related to the Account Export feature.
src/public/favicon.png: Image filesrc/public/pico.colors.min.css: Minified CSSsrc/public/pico.min.css: Minified CSS
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any export-related files in src/public
# that justify this change
# Look for export-related files in public directory
fd . src/public -t f | rg -i "export"
Length of output: 37
Script:
#!/bin/bash
# Let's first check if src/public directory exists and what files it contains
fd . src/public -t f
# Let's also check recent git changes to understand if this directory was added
git log -n 5 --oneline -- src/public/
# Let's check if there are any references to this directory in the PR
git diff HEAD~1 | grep -A 5 -B 5 "src/public"
Length of output: 146
src/components/OtpForm.tsx (2)
2-2: Good type safety improvement.
Restricting the method prop to specific literal values enhances type safety and prevents invalid method assignments.
Line range hint 1-29: Verify component usage in export flow.
Let's verify if this component is used in the account export implementation to understand the relevance of this change to the PR objectives.
compose-fs.yaml (1)
11-13: Validate asset storage configuration
The storage configuration needs validation to ensure proper functionality.
✅ Verification successful
Asset storage configuration is properly set up
The configuration is valid and follows the documented requirements:
- The
DRIVE_DISK: fssetting correctly specifies local filesystem storage - The
FS_ASSET_PATHis properly set to/var/lib/holloand mapped to a named volumeassets_data - The
ASSET_URL_BASEis correctly configured for local serving athttp://localhost:3000/assets/
The code in src/storage.ts validates these settings at runtime and the configuration matches the requirements documented in multiple languages in the docs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the asset storage configuration
# Check if the volume mount point exists and has correct permissions
docker compose -f compose-fs.yaml exec hollo \
sh -c '[ -d "/var/lib/hollo" ] && [ -w "/var/lib/hollo" ] || exit 1'
# Verify the asset URL is accessible
docker compose -f compose-fs.yaml exec hollo \
curl -f http://localhost:3000/assets/ || exit 1
Length of output: 403
Script:
#!/bin/bash
# Let's analyze the configuration and related code instead
# Check compose file configuration
cat compose-fs.yaml
# Search for asset-related configuration and usage
rg -l "DRIVE_DISK|ASSET_URL_BASE|FS_ASSET_PATH" --type yaml --type js --type ts
# Look for volume mounts in compose file
rg "volumes:" -A 5 compose-fs.yaml
# Search for asset storage related code
rg -g '!{test,dist,build}/*' "FS_ASSET_PATH|ASSET_URL_BASE" -A 3
Length of output: 5875
src/components/NewAccountPage.tsx (1)
20-20: 🛠️ Refactor suggestion
Verify handling of required officialAccount prop
The officialAccount is marked as required but there's no default value or error handling. This could cause runtime issues if the prop is not provided.
Consider one of these approaches:
- officialAccount: string;
+ officialAccount?: string;Or add a default value in the component:
<AccountForm
action="/accounts"
values={props.values}
errors={props.errors}
submitLabel="Create a new account"
- officialAccount={props.officialAccount}
+ officialAccount={props.officialAccount ?? "[email protected]"}
/>Also applies to: 35-35
✅ Verification successful
officialAccount prop is properly handled - no changes needed
The verification shows that:
NewAccountPageis always used withofficialAccountprop set toHOLLO_OFFICIAL_ACCOUNT- All usages in
src/pages/accounts.tsxconsistently provide this prop - The prop is correctly typed as required since it's always needed and provided
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for instances where NewAccountPage is used without officialAccount
ast-grep --pattern 'NewAccountPage $props'
Length of output: 44
Script:
#!/bin/bash
# Let's try a more comprehensive search for NewAccountPage usage
rg "NewAccountPage" -A 3 -B 3
# Also search for the component definition to understand its props
ast-grep --pattern 'interface $_ {
officialAccount: string;
$$$
}'
# And search for AccountForm usage to see how officialAccount is handled
rg "officialAccount=" -A 2 -B 2
Length of output: 4092
src/components/LoginForm.tsx (1)
2-2: LGTM! Improved type safety
Good improvement to restrict the method prop to specific valid values, enhancing type safety and preventing potential runtime errors.
drizzle/0053_instances.sql (2)
11-15: Review data migration strategy
The use of any_value() for software and software_version might lead to inconsistent data if different accounts on the same instance have different values. Consider:
- Validating the consistency of these values before migration
- Adding logging or validation of the selected values
✅ Verification successful
The previous command didn't work because we need to look at the actual migration file and related schema to understand the data model better. Let's examine the SQL files to verify our concerns about data consistency during migration.
We need to understand how the software fields were initially added and any existing constraints before making a final assessment about the data consistency concerns during migration.
Migration strategy is appropriate for the data model
The migration strategy is actually sound because:
- The
softwareandsoftware_versionfields were optional columns (no NOT NULL constraint) when added in migration 0048 - These fields logically belong to the instance level, not individual accounts
- The
any_value()usage is appropriate here as software information should be consistent across all accounts from the same instance, and any differences would have been data inconsistencies in the original model
- The migration correctly consolidates instance-level information while maintaining referential integrity through foreign key constraints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for inconsistent software info across accounts
rg -l 'SELECT.*FROM.*accounts.*GROUP BY' | xargs cat
Length of output: 54
Script:
#!/bin/bash
# Let's check the content of the migration file directly
cat drizzle/0053_instances.sql
# Also check for any related schema definitions
fd -e sql . drizzle/
Length of output: 3014
Script:
#!/bin/bash
# Let's check the software field addition migration to understand the data model
cat drizzle/0048_accounts.software.sql
# Also check for any existing constraints or indexes on accounts table
rg -A 5 'CREATE TABLE.*"accounts"' drizzle/
Length of output: 559
8-16:
Validate host extraction logic
The regex pattern '^@[^@]+@' assumes a specific format for handles. Consider:
- Adding validation for the extracted host values
- Handling potential NULL or invalid cases
- Adding a comment explaining the expected handle format
✅ Verification successful
The previous search might have been too specific. Let's broaden our search to understand how handles are used and validated throughout the codebase, including any SQL definitions or patterns that match the format being used in the migration.
Let's focus on the specific files we found in the previous search and examine the handle format implementation and validation.
Based on the code analysis, I can now provide a final response regarding the handle extraction validation concern:
Handle format validation is implemented but could be documented better
The codebase shows that handle format validation is properly implemented:
- The regex pattern
'^@[^@]+@'is consistently used across the application to extract instance hosts from handles - Handle format is validated in
text.tsthrough the mention plugin and account persistence logic - NULL cases are handled by the SQL migration's usage of
regexp_replace
However, to improve maintainability:
- Add a comment in the migration file explaining the expected handle format (e.g.,
@[email protected]) - Consider extracting the regex pattern into a shared constant with documentation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for handle format validation in codebase
rg -A 5 'handle.*validation|validate.*handle'
Length of output: 47
Script:
#!/bin/bash
# Search for handle-related code and patterns
rg -A 5 '@[^@]+@|handle.*format|format.*handle'
# Search for handle-related database operations
ast-grep --pattern 'handle'
# Look for any SQL migrations or schema definitions related to handles
fd -e sql . | xargs cat
Length of output: 65963
Script:
#!/bin/bash
# Search for handle-related code in text.ts which seems to handle the format
cat src/text.ts
# Look for handle validation or format definitions
rg -A 5 'handle.*=|const.*handle|function.*handle'
# Look at the SQL migration file directly
cat drizzle/0053_instances.sql
Length of output: 26429
compose.yaml (2)
3-3: Consider stability implications of using canary tag
Using the canary tag instead of latest might introduce instability in production. Consider using a specific version tag for better predictability and stability.
11-12: Verify storage configuration for account export functionality
The new storage configuration using S3 and local asset URL base aligns with the account export requirements. However, ensure that:
- The export functionality properly handles the S3 storage configuration
- The local asset URL is accessible when running behind a proxy
✅ Verification successful
Let me check the implementation details in these files to verify the storage configuration usage.
Based on the code analysis, I can now generate the final response:
Storage configuration is properly implemented for S3 and asset handling
The codebase shows proper implementation of S3 storage configuration:
- Storage configuration in
storage.tscorrectly handles S3 disk type and asset URL base - Asset URL generation is properly implemented with fallback mechanisms
- Media upload functionality in
api/v1/media.tscorrectly uses the configured storage disk and asset URL base
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for S3 configuration usage in export-related code
rg -l 'DRIVE_DISK|ASSET_URL_BASE' src/
Length of output: 69
Script:
#!/bin/bash
# Check storage.ts implementation for S3 and export handling
rg -A 5 'DRIVE_DISK|ASSET_URL_BASE' src/storage.ts
# Check index.tsx for any relevant configuration usage
rg -A 5 'DRIVE_DISK|ASSET_URL_BASE' src/index.tsx
# Look for export-related functionality
ast-grep --pattern 'export function $FUNC($$$) { $$$ }'
Length of output: 51799
src/components/Layout.tsx (1)
45-48: LGTM: Favicon configuration
The favicon configuration follows standard practices with appropriate attributes.
src/logging.ts (1)
1-1: Verify Sentry dependency installation
The addition of Sentry integration requires:
- Proper installation of @logtape/sentry package
- Sentry configuration in the application
Also applies to: 9-9
✅ Verification successful
Sentry integration is properly configured
The verification confirms:
- @logtape/sentry package is installed (v0.1.0)
- Sentry sink is properly imported and configured in the logging setup
- Sentry sink is correctly assigned to the "fedify" logger category
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Sentry dependencies are properly installed
grep -r "@logtape/sentry" package.json
Length of output: 71
Script:
#!/bin/bash
# Check if Sentry is properly configured in the codebase
rg "@logtape/sentry" -A 5
Length of output: 725
Script:
#!/bin/bash
# Check Sentry configuration and initialization
rg "getSentrySink" -A 10
Length of output: 1071
src/components/SetupForm.tsx (1)
2-2: Type refinement looks good!
The change from string to "get" | "post" | "dialog" union type improves type safety by restricting the method prop to valid values.
Let's verify consistent usage across other form components:
✅ Verification successful
Type refinement is consistently applied across form components ✅
The verification shows that the method prop type is consistently defined as "get" | "post" | "dialog" in all form components:
SetupForm.tsxLoginForm.tsxOtpForm.tsxAccountForm.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for form method prop types in other components
ast-grep --pattern 'interface $name {
$$$
method?: $type;
$$$
}'
Length of output: 72
Script:
#!/bin/bash
# Let's try a different approach to find form method usages
# First, let's find TypeScript/React files that might contain form-related code
rg -t typescript -t tsx "method[?]?:\s*(string|['\"]get['\"]|['\"]post['\"]|['\"]dialog['\"])" --context 2
Length of output: 155
Script:
#!/bin/bash
# Let's try searching for files with .ts or .tsx extension that contain method definitions
rg "method[?]?:\s*(string|['\"]get['\"]|['\"]post['\"]|['\"]dialog['\"])" -g "*.{ts,tsx}" --context 2
Length of output: 1042
docs/src/content/docs/zh-cn/install/env.mdx (2)
40-51: Critical security warning is well documented
The security implications of ALLOW_PRIVATE_ADDRESS are properly emphasized with clear warnings about production usage.
91-107: Well-documented storage configuration with clear migration warning
The storage configuration section effectively explains the options and includes an important caution about migration implications. The backward compatibility note and future changes warning are particularly helpful.
src/entities/exportAccount.ts (4)
166-169: 🛠️ Refactor suggestion
Enhance error handling with specific error types and context.
The current error handling is too generic and lacks useful context for debugging.
- } catch (e) {
- console.error(e);
- return c.json({ error: "Internal server error occurred" }, 500);
+ } catch (error) {
+ console.error('Account export failed:', error);
+ const message = error instanceof Error ? error.message : 'Unknown error';
+ return c.json({
+ error: "Failed to export account data",
+ details: message,
+ actorId: this.actorId
+ }, 500);
}Likely invalid or redundant comment.
10-11: 🛠️ Refactor suggestion
Improve environment variable handling.
The biome-ignore comment lacks explanation and the homeUrl setup could be more robust.
-// biome-ignore lint/complexity/useLiteralKeys: <explanation>
-const homeUrl = process.env["HOME_URL"] || "http://localhost:3000/";
+// biome-ignore lint/complexity/useLiteralKeys: Using bracket notation for consistency with process.env access
+const DEFAULT_HOME_URL = "http://localhost:3000/";
+const homeUrl = process.env["HOME_URL"] || DEFAULT_HOME_URL;
+
+if (!homeUrl.startsWith("http")) {
+ throw new Error("HOME_URL must be a valid URL starting with http:// or https://");
+}Likely invalid or redundant comment.
17-22: 🛠️ Refactor suggestion
Enhance actorId validation in constructor.
The constructor only checks for presence but not format of actorId.
constructor(actorId: string) {
- if (!actorId) {
+ if (!actorId?.trim()) {
throw new Error("Invalid actorId");
}
+ if (!/^[a-zA-Z0-9_-]+$/.test(actorId)) {
+ throw new Error("actorId contains invalid characters");
+ }
this.actorId = actorId;
}Likely invalid or redundant comment.
172-194: 🛠️ Refactor suggestion
Add timeout and retry logic to media downloads.
The media download function should handle timeouts and implement retries for robustness.
+const TIMEOUT_MS = 30000;
+const MAX_RETRIES = 3;
+
private async downloadMedia(
mediaUrl: string,
): Promise<null | { buffer: ArrayBuffer; contentType: string }> {
if (!mediaUrl) {
return null;
}
+ let attempts = 0;
+ while (attempts < MAX_RETRIES) {
try {
- const response = await fetch(mediaUrl);
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), TIMEOUT_MS);
+
+ const response = await fetch(mediaUrl, {
+ signal: controller.signal,
+ });
+ clearTimeout(timeout);
if (!response.ok) {
throw new Error(`Failed to fetch media: ${response.statusText}`);
}
+ const contentType = response.headers.get("Content-Type");
+ if (!contentType) {
+ throw new Error('Missing Content-Type header');
+ }
return {
buffer: await response.arrayBuffer(),
- contentType: response.headers.get("Content-Type") || "application/bin",
+ contentType,
};
} catch (error) {
+ attempts++;
+ if (attempts === MAX_RETRIES) {
console.error(`Error downloading media from ${mediaUrl}:`, error);
return null;
+ }
+ // Exponential backoff
+ await new Promise(resolve => setTimeout(resolve, Math.pow(2, attempts) * 1000));
}
}
+ return null;
}Likely invalid or redundant comment.
docs/src/content/docs/install/env.mdx (1)
Line range hint 1-189: Documentation is well-structured and comprehensive.
The environment variables documentation is clear, organized, and includes appropriate warnings and notes for different configuration options.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~107-~107: Possible missing preposition found.
Context: ...d s3 (S3-compatible object storage). Defaults to s3 for backward compatibility, but...
(AI_HYDRA_LEO_MISSING_IT)
[style] ~108-~108: Style-wise, it’s not ideal to insert an adverb (‘explicitly’) in the middle of an infinitive construction (‘to configure’). Try moving the adverb to avoid split infinitives.
Context: ...` for backward compatibility, but it is recommended to explicitly configure the driver as the default value will be removed in...
(SPLIT_INFINITIVE)
src/entities/status.ts (1)
219-219: LGTM! Verify URL handling consistency
The fallback to post.iri when post.url is null/undefined is a good practice. This ensures posts always have a valid URL for reference.
✅ Verification successful
URL handling is consistently implemented across the codebase
The codebase shows a uniform pattern of using the nullish coalescing operator (??) with url and iri across all UI components and entity handlers. The only exceptions are in federation-related files where URLs are explicitly converted to URL objects, which is the correct approach for federation handling.
Key consistent implementations found in:
- Entity handlers (
status.ts,account.ts) - UI Components (
Post.tsx,Profile.tsx,AccountList.tsx) - Profile pages (
profile/index.tsx,profile/profilePost.tsx) - Home page (
home/index.tsx)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent URL handling patterns across the codebase
# Verify that IRI is consistently used as a fallback for URL
# Search for URL handling patterns
echo "Checking URL handling patterns..."
rg -A 1 "\.url\s*[?]{2}\s*\.iri"
# Search for potential inconsistencies
echo "Checking for potential inconsistencies..."
rg "\.url\s*\?\s*[^.]|\.url\s*\|\|\s*"
Length of output: 1606
src/pages/profile/index.tsx (3)
38-39: LGTM! Robust cursor-based pagination implementation
The implementation correctly handles the continuation token:
- Proper string validation
- Safe type conversion
- Correct query modification using the
lteoperator
Also applies to: 44-44
134-135: LGTM! Clear pagination UI implementation
The pagination UI is implemented cleanly:
- Clear "Older" link text
- Proper URL construction
- Conditional rendering
Also applies to: 268-270
83-127: LGTM! Efficient pinned posts handling
The optimization to only fetch pinned posts on the first page is a good performance improvement.
✅ Verification successful
Let me check the implementation in federation/index.ts to understand how pinned posts are handled there.
Let me try a different approach to check the file content.
Pinned posts handling is consistent across the codebase
The optimization to only fetch pinned posts on the first page in the profile view is correct and doesn't conflict with other implementations. The other usage in federation/index.ts follows a similar pattern for fetching pinned posts with proper ordering and relationships, but serves a different purpose (federation API). Both implementations maintain consistent ordering using desc(pinnedPosts.index).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that pinned posts are consistently handled
# Check for other places where pinned posts are fetched
echo "Checking pinned posts query patterns..."
ast-grep --pattern 'db.query.pinnedPosts.findMany'
Length of output: 300
Script:
#!/bin/bash
# Check the context around pinned posts query in federation/index.ts
echo "Checking federation/index.ts implementation..."
rg -B 5 -A 10 "db\.query\.pinnedPosts\.findMany" federation/index.ts
Length of output: 233
Script:
#!/bin/bash
# Find the correct path for federation-related files
echo "Finding federation-related files..."
fd federation
# Once we find the file, let's check its content
echo -e "\nChecking pinned posts implementation in all files..."
rg -B 5 -A 10 "pinnedPosts\.findMany"
Length of output: 2171
drizzle/meta/0053_snapshot.json (3)
7-82: LGTM! Well-designed access tokens table
The access_tokens table properly implements OAuth2 token storage with:
- Appropriate cascading deletes
- Required scopes tracking
- Proper timestamp handling
2156-2245: LGTM! Comprehensive enum definitions
The enum definitions are well-thought-out and cover all necessary cases:
- Account types align with ActivityPub
- Post visibility levels are appropriate
- OAuth2 scopes are comprehensive
1642-1929: LGTM! Robust posts table design
The posts table implementation is solid with:
- Proper visibility handling
- Content versioning support
- Comprehensive relationship tracking
- Appropriate indexes for common queries
✅ Verification successful
Based on the verification results, I can now generate the final response:
Confirmed: Posts table has robust relationship handling and visibility controls
The verification confirms the posts table implementation is well-designed with:
- Proper visibility handling through
post_visibilityenum type and composite indexes including visibility - Comprehensive relationship tracking with properly indexed foreign keys:
reply_target_idfor replies (SET NULL on delete)sharing_idfor shares (CASCADE on delete)quote_target_idfor quotes (SET NULL on delete)
- Optimized querying through strategic indexes:
posts_sharing_id_indexposts_actor_id_sharing_id_indexposts_reply_target_id_indexposts_visibility_actor_id_sharing_id_indexposts_visibility_actor_id_reply_target_id_index
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify post table usage patterns
# Check for proper visibility handling
echo "Checking post visibility handling..."
rg -A 2 "visibility.*post_visibility"
# Check for proper relationship querying
echo "Checking relationship querying patterns..."
rg "sharing_id|reply_target_id|quote_target_id"
Length of output: 60770
drizzle/meta/0054_snapshot.json (1)
606-617:
Review password hash storage implementation
The credentials table stores password hashes. Ensure:
- Use of strong hashing algorithms (e.g., Argon2, bcrypt)
- Proper salt implementation
- No plaintext password storage
✅ Verification successful
Based on the search results, I can see that the codebase uses Bun's password hashing functionality with Argon2id algorithm for password hashing (Bun.password.hash(password, { algorithm: "argon2id" })), and verification is done using Bun.password.verify(). Let me verify the complete implementation to ensure there are no security concerns.
Password storage implementation is secure
The implementation uses industry-standard security practices:
- Passwords are hashed using Argon2id, a strong and recommended hashing algorithm
- Password verification is done securely using Bun's built-in verify function
- No plaintext password storage found, only hashed values are stored
- Salt is automatically handled by Bun's password hashing implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any potential plaintext password storage
rg -i "password[^_]" --type ts
Length of output: 3061
Script:
#!/bin/bash
# Check for password hash implementation details
ast-grep --pattern 'Bun.password.$_($$$)'
Length of output: 255
drizzle/meta/0055_snapshot.json (1)
1-2296: No significant changes in this snapshot
This snapshot maintains the same schema structure as the previous version.
src/public/pico.min.css
Outdated
| @charset "UTF-8";/*! | ||
| * Pico CSS ✨ v2.0.6 (https://picocss.com) | ||
| * Copyright 2019-2024 - Licensed under MIT |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a package manager to manage Pico CSS
Including minified CSS files directly in the repository can make maintenance and updates more challenging. It's recommended to use a package manager like npm or yarn to manage Pico CSS dependencies. This approach simplifies updates and ensures you're using the latest, secure versions.
src/index.tsx
Outdated
| import { DRIVE_DISK, assetPath } from "./storage"; | ||
|
|
||
| // biome-ignore lint/complexity/useLiteralKeys: tsc complains about this (TS4111) | ||
| configureSentry(process.env["SENTRY_DSN"]); |
There was a problem hiding this comment.
Ensure Sentry is configured only when SENTRY_DSN is provided
Currently, configureSentry(process.env["SENTRY_DSN"]); is called without checking if SENTRY_DSN is defined. If SENTRY_DSN is undefined, this may lead to unexpected behavior or errors. Consider adding a condition to configure Sentry only when a DSN is provided.
Apply this diff to check for the existence of SENTRY_DSN before configuring Sentry:
+const SENTRY_DSN = process.env["SENTRY_DSN"];
+if (SENTRY_DSN) {
+ configureSentry(SENTRY_DSN);
+}📝 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.
| configureSentry(process.env["SENTRY_DSN"]); | |
| const SENTRY_DSN = process.env["SENTRY_DSN"]; | |
| if (SENTRY_DSN) { | |
| configureSentry(SENTRY_DSN); | |
| } |
src/media.ts
Outdated
| throw new Error(`Failed to store thumbnail: ${error.message}`, error); | ||
| } |
There was a problem hiding this comment.
Use the 'cause' option when rethrowing errors
When creating a new Error, using a second argument as the original error is not standard. Instead, use the cause property to include the original error.
Apply this diff to correctly rethrow the error with the original cause:
if (error instanceof Error) {
- throw new Error(`Failed to store thumbnail: ${error.message}`, error);
+ throw new Error(`Failed to store thumbnail: ${error.message}`, { cause: error });
}Committable suggestion skipped: line range outside the PR's diff.
src/api/v1/media.ts
Outdated
| const extension = mime.getExtension(file.type); | ||
| if (!extension) { | ||
| return c.json({ error: "Unsupported media type" }, 400); | ||
| } |
There was a problem hiding this comment.
Restrict and validate allowed media types
Allowing any media type may pose security risks. It's important to validate and restrict the allowed media types to a predefined list.
Apply this diff to validate the media type:
+const allowedMediaTypes = ['image/jpeg', 'image/png', 'video/mp4', 'video/webm'];
+if (!allowedMediaTypes.includes(file.type)) {
+ return c.json({ error: "Unsupported media type" }, 400);
+}
const extension = mime.getExtension(file.type);
if (!extension) {
return c.json({ error: "Unsupported media type" }, 400);
}📝 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 extension = mime.getExtension(file.type); | |
| if (!extension) { | |
| return c.json({ error: "Unsupported media type" }, 400); | |
| } | |
| const allowedMediaTypes = ['image/jpeg', 'image/png', 'video/mp4', 'video/webm']; | |
| if (!allowedMediaTypes.includes(file.type)) { | |
| return c.json({ error: "Unsupported media type" }, 400); | |
| } | |
| const extension = mime.getExtension(file.type); | |
| if (!extension) { | |
| return c.json({ error: "Unsupported media type" }, 400); | |
| } |
src/storage.ts
Outdated
| access( | ||
| assetPath, | ||
| constants.F_OK | constants.R_OK | constants.W_OK, | ||
| (err) => { | ||
| if (err) { | ||
| throw new Error(`${assetPath} must be readable and writable`); | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Correct asynchronous access usage in synchronous context
The access function is asynchronous and uses a callback, but the code proceeds without waiting for the result. This can cause unexpected behavior. Use the synchronous version accessSync instead.
Apply this diff to fix the issue:
- access(
- assetPath,
- constants.F_OK | constants.R_OK | constants.W_OK,
- (err) => {
- if (err) {
- throw new Error(`${assetPath} must be readable and writable`);
- }
- },
- );
+ try {
+ accessSync(
+ assetPath,
+ constants.F_OK | constants.R_OK | constants.W_OK,
+ );
+ } catch (err) {
+ throw new Error(`${assetPath} must be readable and writable`);
+ }📝 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.
| access( | |
| assetPath, | |
| constants.F_OK | constants.R_OK | constants.W_OK, | |
| (err) => { | |
| if (err) { | |
| throw new Error(`${assetPath} must be readable and writable`); | |
| } | |
| }, | |
| ); | |
| try { | |
| accessSync( | |
| assetPath, | |
| constants.F_OK | constants.R_OK | constants.W_OK, | |
| ); | |
| } catch (err) { | |
| throw new Error(`${assetPath} must be readable and writable`); | |
| } |
src/logging.ts
Outdated
| @@ -33,27 +35,37 @@ const stdout = new WritableStream({ | |||
| }); | |||
|
|
|||
| await configure({ | |||
| contextLocalStorage: new AsyncLocalStorage(), | |||
There was a problem hiding this comment.
💡 Codebase verification
Sentry configuration needs enhancement for proper error tracking and PII protection
The current Sentry implementation lacks:
- Error context capture mechanisms
- PII data filtering configuration (no
beforeSendhook) - Explicit sensitive data handling during error reporting
Required improvements:
- Add
beforeSendhook in Sentry initialization to filter sensitive data - Implement proper error context capture using
Sentry.setContext - Configure explicit PII data scrubbing rules
- Add error boundary for the account export functionality
🔗 Analysis chain
Review Sentry configuration for sensitive data
Ensure that:
- Sentry is properly configured to handle sensitive data during account export
- PII is not accidentally logged
- Error context is appropriately captured
Also applies to: 45-45
Let me check the Sentry configuration implementation to verify the handling of sensitive data.
Let me check how errors are handled and what context is being captured with Sentry.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Sentry configuration
rg "Sentry.init|configureSentry" src/
Length of output: 293
Script:
#!/bin/bash
# Check Sentry configuration implementation
cat src/sentry.ts
# Look for PII or sensitive data handling
rg -i "sensitive|pii|beforeSend|sanitize" src/
Length of output: 2269
Script:
#!/bin/bash
# Look for error handling and context capture with Sentry
rg "Sentry\.(captureException|captureMessage|setContext|setTag|setExtra)" src/
# Check for AsyncLocalStorage usage with Sentry
rg -A 3 "contextLocalStorage" src/
Length of output: 312
src/entities/exportAccount.ts
Outdated
| const column = type === "following" ? "followingId" : "followerId"; | ||
| const column = type === "following" ? "followingId" : "followerId"; |
There was a problem hiding this comment.
Fix duplicate variable declaration.
The column variable is declared twice, which is a syntax error.
-const column = type === "following" ? "followingId" : "followerId";
-const column = type === "following" ? "followingId" : "followerId";
+const column = type === "following" ? "followingId" : "followerId";📝 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 column = type === "following" ? "followingId" : "followerId"; | |
| const column = type === "following" ? "followingId" : "followerId"; | |
| const column = type === "following" ? "followingId" : "followerId"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Shouldn't redeclare 'column'. Consider to delete it or rename it.
'column' is defined here:
(lint/suspicious/noRedeclare)
src/entities/exportAccount.ts
Outdated
| orderedItems: followingAccounts.map((account) => ({ | ||
| account: this.normalizeUrl(`accounts/${account.followingId}`), | ||
| account: this.normalizeUrl(`accounts/${account.followingId}`), | ||
| showBoosts: account.shares, |
There was a problem hiding this comment.
Remove duplicate account property.
The account property is defined twice in the object literal.
orderedItems: followingAccounts.map((account) => ({
account: this.normalizeUrl(`accounts/${account.followingId}`),
- account: this.normalizeUrl(`accounts/${account.followingId}`),
showBoosts: account.shares,📝 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.
| orderedItems: followingAccounts.map((account) => ({ | |
| account: this.normalizeUrl(`accounts/${account.followingId}`), | |
| account: this.normalizeUrl(`accounts/${account.followingId}`), | |
| showBoosts: account.shares, | |
| orderedItems: followingAccounts.map((account) => ({ | |
| account: this.normalizeUrl(`accounts/${account.followingId}`), | |
| showBoosts: account.shares, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 77-77: This property value named account is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named account
(lint/suspicious/noDuplicateObjectKeys)
drizzle/meta/0054_snapshot.json
Outdated
| "rsa_private_key_jwk": { | ||
| "name": "rsa_private_key_jwk", | ||
| "type": "jsonb", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, | ||
| "rsa_public_key_jwk": { | ||
| "name": "rsa_public_key_jwk", | ||
| "type": "jsonb", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, | ||
| "ed25519_private_key_jwk": { | ||
| "name": "ed25519_private_key_jwk", | ||
| "type": "jsonb", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, | ||
| "ed25519_public_key_jwk": { | ||
| "name": "ed25519_public_key_jwk", | ||
| "type": "jsonb", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, |
There was a problem hiding this comment.
Consider encrypting sensitive cryptographic keys
The account_owners table stores private keys (RSA and Ed25519) as JSONB. While JSONB provides flexibility, storing private keys in plaintext poses a security risk. Consider:
- Encrypting private keys at rest using a strong encryption algorithm
- Using PostgreSQL's pgcrypto extension
- Storing keys in a dedicated key management service
drizzle/meta/0056_snapshot.json
Outdated
| "idempotence_key": { | ||
| "name": "idempotence_key", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding an index for idempotence_key
The new idempotence_key column is used to prevent duplicate post submissions, but lacks an index. This could impact performance when checking for existing keys during post creation.
Consider adding a unique index to ensure both uniqueness and query performance:
CREATE UNIQUE INDEX posts_idempotence_key_idx ON posts (idempotence_key) WHERE idempotence_key IS NOT NULL;The partial index (WHERE clause) ensures that:
- Only non-null values are indexed
- Multiple posts can have NULL idempotence_key
- Efficient lookups when checking for duplicates
💡 Codebase verification
Idempotency implementation needs improvements
The current implementation has some gaps that need to be addressed:
- The idempotency key is accepted from the
Idempotency-Keyheader but there's no validation of its format or strength - While there's a 1-hour cleanup window (
CURRENT_TIMESTAMP - INTERVAL '1 hour'), there's no periodic cleanup mechanism - The application uses crypto functions elsewhere but not for generating idempotency keys
Required changes:
- Validate the provided idempotency key format and strength
- Add a background job or trigger for cleaning up old idempotency keys
- Consider generating secure idempotency keys server-side if none provided
🔗 Analysis chain
Verify idempotence implementation in application code
The idempotence_key column is a good addition for handling duplicate submissions. Ensure the application code:
- Generates cryptographically secure keys
- Implements proper error handling for duplicate key violations
- Cleans up old keys periodically
Let me search for more context around the implementation.
Let me check how the idempotency key is generated and validated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check idempotence key implementation
rg -A 5 "idempotence.*key" --type ts
Length of output: 344
Script:
#!/bin/bash
# Check for idempotence related code
rg -A 10 "idempotence" --type ts
# Look for insert operations that might use the idempotence key
ast-grep --pattern 'insert($$$).values($$$)'
Length of output: 1950
Script:
#!/bin/bash
# Check for idempotency key generation and validation
rg -B 5 -A 10 "idempotencyKey" --type ts
# Look for crypto or random related imports
rg "crypto" --type ts
Length of output: 3738
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
CHANGES.md (2)
3-9: Enhance changelog entry for the Account Export featureThe changelog entry for version 0.4.0 could be more specific about the new Account Export API endpoint. Consider adding details about:
- The new API endpoint path
- Authentication requirements
- Export format details (beyond just mentioning CSV)
## Version 0.4.0 To be released. - - Now you can import and export your data from the administration dashboard - in CSV format: follows, lists, accounts you muted, accounts you blocked, - and bookmarks. + - Added new Account Export API endpoint (`/:actor/accountExport`) that allows users to: + - Export account data through the administration dashboard + - Download data in CSV format including: follows, lists, muted accounts, + blocked accounts, and bookmarks + - Requires authentication with appropriate scope
155-158: Fix word repetition in changelog entryThere's a word repetition in the changelog entry that should be corrected.
- Fixed a bug where followers-only posts from accounts that had had set + Fixed a bug where followers-only posts from accounts that had set🧰 Tools
🪛 LanguageTool
[duplication] ~155-~155: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...(ENGLISH_WORD_REPEAT_RULE)
[style] ~158-~158: Consider using a different verb for a more formal wording.
Context: ... from the administration dashboard to fix the issue. - Fixed the federated (publ...(FIX_RESOLVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (2)
CHANGES.md(1 hunks)package.json(3 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGES.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...S_ASSET_PATHenvironment variable. - AddedASSET_URL_BASE` environment variable t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d SENTRY_DSN environment variable. - Added pagination to the profile page. [[#40]]...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~107-~107: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...shboard to Federation. - Now posts also can be force-refreshed. - Now the number ...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[duplication] ~155-~155: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~158-~158: Consider using a different verb for a more formal wording.
Context: ... from the administration dashboard to fix the issue. - Fixed the federated (publ...
(FIX_RESOLVE)
🔇 Additional comments (2)
package.json (2)
64-64: Verify Bun version compatibility with new dependencies
The Bun version requirement has been updated to ^1.1.38. Ensure this version is compatible with all new dependencies, particularly @sentry/bun and @interop/wallet-export-ts.
4-4: Verify dependency versions and security implications
The new dependencies align well with the Account Export API implementation. However, some considerations:
- The addition of
@interop/wallet-export-tsandflydrivesupports the export functionality. - Sentry integration (
@sentry/bun,@sentry/core,@logtape/sentry) suggests proper error tracking is being implemented.
Let's verify the security implications of these dependencies:
Also applies to: 16-16, 17-17, 20-20, 23-23, 25-29, 35-35
✅ Verification successful
Let me gather more information about the specific versions being used in the package.json and check other dependencies as well.
Dependencies are secure and well-maintained
The security verification shows positive results:
- No known vulnerabilities found in
flydrive,@sentry/core, and other new dependencies @interop/wallet-export-tsis actively maintained with regular updates (last update: Dec 12, 2024)- All critical dependencies use fixed major versions with flexible minor updates (e.g.,
^8.41.0for Sentry packages) - Security-critical packages like
xss,ssrfcheck, andsemverare pinned to stable versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for known vulnerabilities in new dependencies
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "flydrive") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
# Check if @interop/wallet-export-ts is actively maintained
curl -s https://registry.npmjs.org/@interop/wallet-export-ts | jq '.time'
Length of output: 681
Script:
#!/bin/bash
# Check package.json for specific versions
cat package.json | jq '.dependencies'
# Check for vulnerabilities in Sentry packages
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "@sentry/core") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
# Check if @logtape/sentry is actively maintained
curl -s https://registry.npmjs.org/@logtape/sentry | jq '.time'
Length of output: 1742
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
CHANGES.md (1)
163-163: Fix word repetition in the changelog.There's a duplicate word "had" in the sentence.
-Fixed a bug where followers-only posts from accounts that had had set +Fixed a bug where followers-only posts from accounts that had set🧰 Tools
🪛 LanguageTool
[duplication] ~163-~163: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...(ENGLISH_WORD_REPEAT_RULE)
src/entities/exportAccount.ts (2)
136-153: Enhance media processing with progress tracking and better error handling.The media processing could benefit from progress tracking and more detailed error handling.
-const mediaPromises = postsData.flatMap((post) => { +const mediaPromises = postsData.flatMap((post, postIndex) => { if (!post.media) return []; - return post.media.map(async (media: { id: string; url: string }) => { + return post.media.map(async (media: { id: string; url: string }, mediaIndex) => { try { + console.log(`Processing media ${mediaIndex + 1}/${post.media.length} for post ${postIndex + 1}/${postsData.length}`); const mediaRecord = await this.downloadMedia(media.url); if (!mediaRecord) return; const extension = mediaRecord.contentType?.split("/")[1]; + if (!extension) { + throw new Error(`Invalid content type for media: ${media.id}`); + } const fileName = `${media.id}.${extension}`; addMediaFile(fileName, mediaRecord.buffer, mediaRecord.contentType); } catch (error) { - console.error(`Error downloading media: ${media.id}`, error); + const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + console.error(`Error processing media ${media.id}: ${errorMessage}`); } }); });
166-169: Enhance error handling with more specific error messages.The current error handling could provide more context about what failed.
- } catch (e) { - console.error(e); - return c.json({ error: "Internal server error occurred" }, 500); + } catch (error) { + console.error('Account export failed:', error); + const message = error instanceof Error ? error.message : 'Unknown error'; + return c.json({ + error: "Failed to export account data", + details: message, + actorId: this.actorId + }, 500); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGES.md(2 hunks)src/entities/exportAccount.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/entities/exportAccount.ts
[error] 77-77: This property value named account is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named account
(lint/suspicious/noDuplicateObjectKeys)
🪛 LanguageTool
CHANGES.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...S_ASSET_PATHenvironment variable. - AddedASSET_URL_BASE` environment variable t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d SENTRY_DSN environment variable. - Added pagination to the profile page. [[#40]]...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~107-~107: Possible missing comma found.
Context: ...tration dashboard to Federation. - Now posts also can be force-refreshed. - ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: A different word order might sound more natural.
Context: ...shboard to Federation. - Now posts also can be force-refreshed. - Now the number ...
(AI_HYDRA_LEO_WORD_ORDER)
[duplication] ~163-~163: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~166-~166: Consider using a different verb for a more formal wording.
Context: ... from the administration dashboard to fix the issue. - Fixed the federated (publ...
(FIX_RESOLVE)
[uncategorized] ~193-~193: Possible missing comma found.
Context: ...e. This caused inoperability with some software such as GoToSocial. - It now sends D...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (4)
CHANGES.md (1)
7-9: LGTM! The changelog entry accurately reflects the new account export feature.
The changelog entry aligns well with the PR objectives for implementing the Account Export API endpoint.
src/entities/exportAccount.ts (3)
77-78:
Remove duplicate property in object literal.
The account property is defined twice in the object literal, causing the first definition to be ignored.
orderedItems: followingAccounts.map((account) => ({
account: this.normalizeUrl(`accounts/${account.followingId}`),
- account: this.normalizeUrl(`accounts/${account.followingId}`),
showBoosts: account.shares,Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 77-77: This property value named account is later overwritten by an object member with the same name.
Overwritten with this value.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property value named account
(lint/suspicious/noDuplicateObjectKeys)
10-11: 🛠️ Refactor suggestion
Improve environment variable handling.
The biome-ignore comment lacks explanation and the homeUrl setup could be more robust.
-// biome-ignore lint/complexity/useLiteralKeys: <explanation>
-const homeUrl = process.env["HOME_URL"] || "http://localhost:3000/";
+// biome-ignore lint/complexity/useLiteralKeys: Using bracket notation for consistency with process.env access
+const DEFAULT_HOME_URL = "http://localhost:3000/";
+const homeUrl = process.env["HOME_URL"] || DEFAULT_HOME_URL;
+
+if (!homeUrl.startsWith("http")) {
+ throw new Error("HOME_URL must be a valid URL starting with http:// or https://");
+}Likely invalid or redundant comment.
172-194: 🛠️ Refactor suggestion
Add timeout and retry logic to media downloads.
The media download function should handle timeouts and implement retry logic for better reliability.
+const TIMEOUT_MS = 30000;
+const MAX_RETRIES = 3;
+
private async downloadMedia(
mediaUrl: string,
): Promise<null | { buffer: ArrayBuffer; contentType: string }> {
if (!mediaUrl) {
return null;
}
+ let attempts = 0;
+ while (attempts < MAX_RETRIES) {
try {
- const response = await fetch(mediaUrl);
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), TIMEOUT_MS);
+
+ const response = await fetch(mediaUrl, {
+ signal: controller.signal,
+ });
+ clearTimeout(timeout);
+
if (!response.ok) {
throw new Error(`Failed to fetch media: ${response.statusText}`);
}
+ const contentType = response.headers.get("Content-Type");
+ if (!contentType) {
+ throw new Error('Missing Content-Type header');
+ }
return {
buffer: await response.arrayBuffer(),
- contentType: response.headers.get("Content-Type") || "application/bin",
+ contentType,
};
} catch (error) {
+ attempts++;
+ if (attempts === MAX_RETRIES) {
console.error(`Error downloading media from ${mediaUrl}:`, error);
return null;
+ }
+ // Exponential backoff
+ await new Promise(resolve => setTimeout(resolve, Math.pow(2, attempts) * 1000));
}
}
+ return null;
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
src/entities/exportAccount.ts (1)
99-169: Implement streaming for large exportsThe current implementation loads all data into memory before creating the export. For large accounts, this could cause memory issues.
Consider implementing a streaming approach:
- Stream posts in batches
- Process media files in chunks
- Use async iterators for better memory management
Would you like me to help create a GitHub issue to track this enhancement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/entities/exportAccount.ts(1 hunks)
🔇 Additional comments (2)
src/entities/exportAccount.ts (2)
10-11: 🛠️ Refactor suggestion
Add validation for HOME_URL environment variable
The HOME_URL environment variable should be validated to ensure it's a valid URL.
+const isValidUrl = (url: string): boolean => {
+ try {
+ new URL(url);
+ return true;
+ } catch {
+ return false;
+ }
+};
+
const homeUrl = process.env["HOME_URL"] || "http://localhost:3000/";
+if (!isValidUrl(homeUrl)) {
+ throw new Error(`Invalid HOME_URL: ${homeUrl}`);
+}Likely invalid or redundant comment.
14-22: 🛠️ Refactor suggestion
Enhance actorId validation and encapsulation
The actorId property should be private, and its validation should be more thorough.
export class AccountExporter {
- actorId: string;
+ private readonly actorId: string;
constructor(actorId: string) {
- if (!actorId) {
+ if (!actorId?.trim()) {
throw new Error("Invalid actorId");
}
+ // Validate actorId format (assuming it should be a UUID or similar)
+ if (!/^[a-zA-Z0-9-_]+$/.test(actorId)) {
+ throw new Error("Invalid actorId format");
+ }
this.actorId = actorId;
}Likely invalid or redundant comment.
| async loadAccount() { | ||
| return db.query.accounts.findFirst({ | ||
| where: eq(schema.accounts.id, this.actorId), | ||
| with: { owner: true }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add error handling for database operations
Database operations should have proper error handling to prevent unhandled exceptions.
async loadAccount() {
+ try {
return db.query.accounts.findFirst({
where: eq(schema.accounts.id, this.actorId),
with: { owner: true },
});
+ } catch (error) {
+ console.error('Error loading account:', error);
+ throw new Error('Failed to load account data');
+ }
}Apply similar error handling to other database query methods.
Also applies to: 31-36, 38-44, 46-50, 52-56
| async loadPosts() { | ||
| return db.query.posts.findMany({ | ||
| where: eq(schema.posts.accountId, this.actorId), | ||
| with: getPostRelations(this.actorId), // Fetch related data using getPostRelations | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add pagination for posts query
Loading all posts at once could cause memory issues with large accounts.
- async loadPosts() {
+ async loadPosts(limit = 100, offset = 0) {
return db.query.posts.findMany({
where: eq(schema.posts.accountId, this.actorId),
with: getPostRelations(this.actorId),
+ limit,
+ offset,
});
}Committable suggestion skipped: line range outside the PR's diff.
| private async downloadMedia( | ||
| mediaUrl: string, | ||
| ): Promise<null | { buffer: ArrayBuffer; contentType: string }> { | ||
| if (!mediaUrl) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch(mediaUrl); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch media: ${response.statusText}`); | ||
| } | ||
|
|
||
| return { | ||
| buffer: await response.arrayBuffer(), | ||
| contentType: response.headers.get("Content-Type") || "application/bin", | ||
| }; // Binary data | ||
| } catch (error) { | ||
| console.error(`Error downloading media from ${mediaUrl}:`, error); | ||
| return null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add security checks for media downloads
The media download function should include security checks to prevent potential vulnerabilities.
private async downloadMedia(
mediaUrl: string,
): Promise<null | { buffer: ArrayBuffer; contentType: string }> {
if (!mediaUrl) {
return null;
}
+
+ // Validate URL protocol
+ const url = new URL(mediaUrl);
+ if (!['http:', 'https:'].includes(url.protocol)) {
+ console.error(`Invalid protocol in URL: ${mediaUrl}`);
+ return null;
+ }
+
+ // Add size limit for media downloads
+ const MAX_SIZE = 50 * 1024 * 1024; // 50MB
try {
const response = await fetch(mediaUrl);
if (!response.ok) {
throw new Error(`Failed to fetch media: ${response.statusText}`);
}
+ const contentLength = response.headers.get('Content-Length');
+ if (contentLength && parseInt(contentLength) > MAX_SIZE) {
+ throw new Error('Media file too large');
+ }
+
return {
buffer: await response.arrayBuffer(),
contentType: response.headers.get("Content-Type") || "application/bin",
};
} catch (error) {
console.error(`Error downloading media from ${mediaUrl}:`, error);
return null;
}
}📝 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.
| private async downloadMedia( | |
| mediaUrl: string, | |
| ): Promise<null | { buffer: ArrayBuffer; contentType: string }> { | |
| if (!mediaUrl) { | |
| return null; | |
| } | |
| try { | |
| const response = await fetch(mediaUrl); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch media: ${response.statusText}`); | |
| } | |
| return { | |
| buffer: await response.arrayBuffer(), | |
| contentType: response.headers.get("Content-Type") || "application/bin", | |
| }; // Binary data | |
| } catch (error) { | |
| console.error(`Error downloading media from ${mediaUrl}:`, error); | |
| return null; | |
| } | |
| } | |
| } | |
| private async downloadMedia( | |
| mediaUrl: string, | |
| ): Promise<null | { buffer: ArrayBuffer; contentType: string }> { | |
| if (!mediaUrl) { | |
| return null; | |
| } | |
| // Validate URL protocol | |
| const url = new URL(mediaUrl); | |
| if (!['http:', 'https:'].includes(url.protocol)) { | |
| console.error(`Invalid protocol in URL: ${mediaUrl}`); | |
| return null; | |
| } | |
| // Add size limit for media downloads | |
| const MAX_SIZE = 50 * 1024 * 1024; // 50MB | |
| try { | |
| const response = await fetch(mediaUrl); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch media: ${response.statusText}`); | |
| } | |
| const contentLength = response.headers.get('Content-Length'); | |
| if (contentLength && parseInt(contentLength) > MAX_SIZE) { | |
| throw new Error('Media file too large'); | |
| } | |
| return { | |
| buffer: await response.arrayBuffer(), | |
| contentType: response.headers.get("Content-Type") || "application/bin", | |
| }; | |
| } catch (error) { | |
| console.error(`Error downloading media from ${mediaUrl}:`, error); | |
| return null; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
CHANGES.md (1)
34-38: Improve changelog formatting and readabilityA few style improvements would enhance the changelog's readability:
- Reduce repetition in environment variable descriptions:
- Added `DRIVE_DISK` environment variable. - Added `FS_ASSET_PATH` environment variable. - Added `ASSET_URL_BASE` environment variable to replace `S3_URL_BASE`. - Deprecated `S3_URL_BASE` environment variable in favor of + Added environment variables: + - `DRIVE_DISK` + - `FS_ASSET_PATH` + - `ASSET_URL_BASE` (replaces `S3_URL_BASE`) + Deprecated environment variables: + - `S3_URL_BASE` (replaced by `ASSET_URL_BASE`)
- Fix word repetition and add missing commas:
- Fixed the last page in the profile using Moshidon leading to infinite + Fixed the last page in the profile, when using Moshidon, leading to infinite - followers-only posts from accounts that had had set + followers-only posts from accounts that had setAlso applies to: 163-164, 202-203
🧰 Tools
🪛 LanguageTool
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...S_ASSET_PATHenvironment variable. - AddedASSET_URL_BASE` environment variable t...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGES.md(1 hunks)package.json(1 hunks)src/components/AccountList.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/AccountList.tsx
🧰 Additional context used
🪛 LanguageTool
CHANGES.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...S_ASSET_PATHenvironment variable. - AddedASSET_URL_BASE` environment variable t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~44-~44: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d SENTRY_DSN environment variable. - Added pagination to the profile page. [[#40]]...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~107-~107: Possible missing comma found.
Context: ...tration dashboard to Federation. - Now posts also can be force-refreshed. - ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: A different word order might sound more natural.
Context: ...shboard to Federation. - Now posts also can be force-refreshed. - Now the number ...
(AI_HYDRA_LEO_WORD_ORDER)
[duplication] ~163-~163: Possible typo: you repeated a word
Context: ...followers-only posts from accounts that had had set their follower lists to private h...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~166-~166: Consider using a different verb for a more formal wording.
Context: ... from the administration dashboard to fix the issue. - Fixed the federated (publ...
(FIX_RESOLVE)
[uncategorized] ~202-~202: Possible missing comma found.
Context: ...ixed the last page in the profile using Moshidon leading to infinite pagination. [[#48...
(AI_HYDRA_LEO_MISSING_COMMA)
| - On creating a new account, the user now can choose to follow the official | ||
| Hollo account. | ||
|
|
||
| - Added `PORT` and `ALLOW_PRIVATE_ADDRESS` environment variables. | ||
| [[#53] by Helge Krueger] | ||
| - Added a favicon. | ||
|
|
||
| - Added `PORT` and `ALLOW_PRIVATE_ADDRESS` environment variables. | ||
| [[#53] by Helge Krueger] |
There was a problem hiding this comment.
Remove duplicate changelog entries
These lines are exact duplicates of the entries at lines 127-133. Remove this redundant section to maintain changelog clarity.
- On creating a new account, the user now can choose to follow the official
- Hollo account.
-
- Added a favicon.
-
- Added `PORT` and `ALLOW_PRIVATE_ADDRESS` environment variables.
- [[#53] by Helge Krueger]| - Now you can import and export your data from the administration dashboard | ||
| in CSV format: follows, lists, accounts you muted, accounts you blocked, | ||
| and bookmarks. |
There was a problem hiding this comment.
Clarify the export format specifications
The current description mentions CSV format for specific data types, but according to the PR objectives, the Account Export API endpoint will generate .tar files containing Actor profiles and related data. Consider updating the changelog to accurately reflect both export formats and their specific use cases.
Suggested revision:
- Now you can import and export your data from the administration dashboard
- in CSV format: follows, lists, accounts you muted, accounts you blocked,
- and bookmarks.
+ Now you can export your data in multiple formats:
+ - CSV format from the administration dashboard: follows, lists, muted accounts,
+ blocked accounts, and bookmarks
+ - Complete account export via API (/:actor/accountExport) including profile,
+ content, follows, and followers in .tar format📝 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.
| - Now you can import and export your data from the administration dashboard | |
| in CSV format: follows, lists, accounts you muted, accounts you blocked, | |
| and bookmarks. | |
| Now you can export your data in multiple formats: | |
| - CSV format from the administration dashboard: follows, lists, muted accounts, | |
| blocked accounts, and bookmarks | |
| - Complete account export via API (/:actor/accountExport) including profile, | |
| content, follows, and followers in .tar format |
This is an in-progress PR aiming to implement FEP-9091: Account Export Service Endopint and FEP-6fcd: Account Export Container Format.
/:actor/accountExportAPI endpointtokenRequired/scopeRequired()like the other endpoints, see the CodeRabbitAI comments)exportActorProfile()to return a ReadStream of a.tarfile as the API responseExample
bookmarks.jsonMastodon exports Bookmarks just as an ActivityPub
OrderedCollection, that has an array of URLs bookmarked.It would be great if some more metadata was stored (like, when something was bookmarked), but that's for future work.
{ "@context":"https://www.w3.org/ns/activitystreams", "id":"bookmarks.json", "type":"OrderedCollection", "orderedItems":[ "https://mastodon.social/users/chrishudsonjr/statuses/107191966598364980", "https://beach.city/users/vantablack/statuses/109275739065882925", "https://sueden.social/users/angel_undercover/statuses/109284854384694363", "https://researchbuzz.masto.host/users/researchbuzz/statuses/109304794799332192", "https://mastodon.gamedev.place/users/saint11/statuses/109309870308504833", "https://pagan.plus/users/Cassana/statuses/109336771390982729", "https://octodon.social/users/quinn/statuses/109344858531708231", "https://mstdn.social/users/OpenArchive/statuses/109348802781496847" ] }Example Following Accounts
Mastodon exports 'Following' list as a CSV (comma-separated values) file, that has the following headers:
We should convert this format to JSON. so, let's do:
{ "@context": "https://www.w3.org/ns/activitystreams", "id":"following_accounts.json", "type":"OrderedCollection", "orderedItems":[ { "account": "[email protected]", "showBoosts": true, "notifyOnNewPosts": true, "language": "..." } ] }Example Followers
Mastodon does NOT, currently, export Followers collection, in any format. So, we're on our own here, in terms of how to structure this.
Let's also make it an
OrderedCollection(like Following above). Do your best in terms of structure (capture the columns in the 'followers' table).Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores