Skip to content

feature: share folders via expiring public links - #277

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1789402355-folder-share-links
Open

devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1789402355-folder-share-links

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 14, 2026

Copy link
Copy Markdown

Summary

Adds "share folder with expiring link": a folder owner mints a tokenized public URL that resolves to a read-only folder listing until it expires or is revoked.

file-service (Rust) — new FolderShareLink { id, folder_id, owner_id, token, expires_at, created_at, revoked } persisted in a new DynamoDB table otterworks-folder-share-links (hash key token, plus ttl_epoch for DynamoDB TTL). Routes under /api/v1/folders:

POST   /{folder_id}/share-links            owner only; body {expires_in_hours: 1..=720} -> 201 {link, url}
GET    /{folder_id}/share-links            owner only; active (non-revoked, non-expired) links
DELETE /{folder_id}/share-links/{link_id}  owner only; soft-revoke
GET    /shared/{token}                     public; 404 unknown/revoked, 410 expired, 200 {folder, files, expires_at}

/shared/{token} is registered before /{folder_id} so it isn't shadowed. URL is built from PUBLIC_WEB_URL (default http://localhost:3000) as /shared/folder/{token}.

api-gateway (Go)/api/v1/folders/shared added to DefaultPrefixPaths so the public resolve skips JWT; /api/v1/folders/{id}/... stays protected (tests added).

"Migration" — the store is DynamoDB, so the migration is the table definition: scripts/localstack-init.sh (local) and infrastructure/terraform/modules/database + IAM ARN + outputs (cloud). New file-service env: DYNAMODB_FOLDER_SHARE_LINKS_TABLE, PUBLIC_WEB_URL (wired in docker-compose.yml). Helm was intentionally not touched (upstream-deployed per AGENTS.md); Terraform changes need sign-off before applying since the tables are shared.

FrontendFolderShareDialog (wired to the existing folder card Share action): expiry select (1h/24h/7d/30d), create, copy, revoke, active-link list. New public route /shared/folder/:tokenSharedFolderPage using a bare axios call (no auth interceptor/redirect) with loading / expired / not-found / error / listing states.

Verification

  • cargo build/test/clippy -D warnings/fmt, npm run lint && npm run build pass. Go tests and terraform fmt/validate were not run locally (toolchains unavailable in this env) — relying on CI.
  • Full local stack (docker compose core profile + LocalStack) exercised via curl and in the browser.

Screenshots

Share dialog opened from folder menu (existing 1h link listed):
share dialog

Creating a 7-day link:
create link

Revoking the 1h link:
revoke

Copied link opened in an incognito (unauthenticated) window:
public shared folder

Revoked link:
revoked

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/18e2e21d6abc4b57b9ffcdf74e517ef2
Open in Devin Desktop: https://app.devin.ai/desktop/session/18e2e21d6abc4b57b9ffcdf74e517ef2?variant=devin
Requested by: @rushcromer


Note

Devin errored when opening this Pull Request as rushcromer.
As a fallback, Devin opened this PR as itself.


Devin Review

Co-Authored-By: Rush Cromer II <rush.cromerii@cognition.ai>
@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 14, 2026 16:35
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 9 potential issues.

Devin Review

Comment on lines +105 to +106
dynamodb_folder_share_links_table: env::var("DYNAMODB_FOLDER_SHARE_LINKS_TABLE")
.unwrap_or_else(|_| "otterworks-folder-share-links".into()),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Cloud share links use nonexistent table

Cloud deployments leave dynamodb_folder_share_links_table at its local default. Both deployment paths omit the suffixed Terraform table, so every share-link request fails.

Learn more

Terraform names the cloud table otterworks-folder-share-links-<environment>, while this default names otterworks-folder-share-links. Existing DynamoDB tables avoid this mismatch because the deployment scripts read each Terraform output and inject the resulting table name. Neither the shared output loader nor the tenant output loader reads dynamodb_folder_share_links_table, and neither file-service Helm argument block injects DYNAMODB_FOLDER_SHARE_LINKS_TABLE. The service therefore sends create, list, revoke, and public-resolve operations to a table absent from the cloud account.

Example: In environment dev, Terraform creates otterworks-folder-share-links-dev. The pod receives no override, opens otterworks-folder-share-links, and a link creation returns ResourceNotFoundException instead of 201.

Recommended fix: Load dynamodb_folder_share_links_table in both deployment output loaders and pass it as config.DYNAMODB_FOLDER_SHARE_LINKS_TABLE in both file-service Helm argument blocks. Ensure the upstream deployment source receives the same wiring before enabling this feature.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. Cloud wiring lives in scripts/deploy-dev.sh / scripts/lib/tenant-common.sh and the Helm values, which per AGENTS.md ship from upstream — intentionally left out of this PR pending user sign-off; flagged to the requester.

Comment thread infrastructure/terraform/modules/database/main.tf Outdated
Comment on lines +69 to +70
public_web_url: env::var("PUBLIC_WEB_URL")
.unwrap_or_else(|_| "http://localhost:3000".into()),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Cloud links point to localhost

Cloud deployments leave public_web_url at http://localhost:3000. Every generated link sends recipients to their own machine instead of OtterWorks.

Learn more

The server embeds public_web_url into every returned share URL. Docker Compose overrides the default, but both Kubernetes deployment paths omit PUBLIC_WEB_URL from their file-service Helm arguments. Kubernetes therefore preserves the localhost fallback even when the frontend is exposed through the shared ingress.

Example: A folder owner on https://t-alice.example.com creates a link. The API returns http://localhost:3000/shared/folder/<token>, so an external recipient opens their own localhost and cannot reach the folder.

Recommended fix: Derive the public frontend origin in both deployment paths and inject it as config.PUBLIC_WEB_URL. Use https://app.${GOLDEN_HOST_SUFFIX} for the golden host, the tenant web host for host routing, and include the tenant base path for path-routing deployments.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above — PUBLIC_WEB_URL injection belongs in the upstream-deployed deploy scripts; deferred pending sign-off. Local compose sets it.

Comment thread services/file-service/src/handlers.rs
Comment on lines +191 to +192
module.database.folder_share_links_table_arn,
"${module.database.folder_share_links_table_arn}/index/*",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Shared infrastructure approval is unrecorded

The new DynamoDB table and IRSA permissions affect shared AWS resources. AGENTS.md requires explicit user sign-off, which the PR context does not record.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

"/health",
"/metrics",
"/socket.io",
"/api/v1/folders/shared",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Public bypass remains boundary-scoped

Boundary-aware matching exposes only /api/v1/folders/shared and descendants. The proxy also removes client-supplied identity headers from these unauthenticated requests.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +332 to +338
getSharedFolder: async (token: string): Promise<SharedFolderView> => {
const { data } = await axios.get<{
folder: Record<string, unknown>;
files: Array<Record<string, unknown>>;
expires_at: string;
}>(`${API_BASE_URL}/folders/shared/${encodeURIComponent(token)}`);
return mapSharedFolderView(data);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Bare Axios mapping is internally consistent

The bare client preserves snake_case fields, which mapSharedFolderView maps explicitly. It also avoids the authenticated client's token verification and redirect behavior.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread services/file-service/src/handlers.rs Outdated
"/health",
"/metrics",
"/socket.io",
"/api/v1/folders/shared",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Public exemption covers all descendants

The exemption makes every method and descendant under /api/v1/folders/shared public. New routes inherit an authentication bypass without explicit configuration.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional: the file-service only registers GET /shared/{token} under that prefix, and the gateway strips identity headers on public paths. Any future route under /shared would need to be added deliberately.

Co-Authored-By: Rush Cromer II <rush.cromerii@cognition.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants