Skip to content

chore: clean up proxy config and remove unused test script#16

Merged
larryro merged 1 commit into
mainfrom
code-cleanup
Dec 14, 2025
Merged

chore: clean up proxy config and remove unused test script#16
larryro merged 1 commit into
mainfrom
code-cleanup

Conversation

@larryro
Copy link
Copy Markdown
Collaborator

@larryro larryro commented Dec 14, 2025

  • Remove ACME_EMAIL from README example (no longer needed)
  • Delete unused services/db/test-setup.sh test script
  • Remove debug-level global config block from Caddyfile
  • Change Caddyfile log level from DEBUG to INFO

Summary by CodeRabbit

  • Chores
    • Removed default environment variable from configuration documentation
    • Reduced logging verbosity by adjusting log level settings
    • Removed database test setup script from repository

✏️ Tip: You can customize this high-level summary in your review settings.

- Remove ACME_EMAIL from README example (no longer needed)
- Delete unused services/db/test-setup.sh test script
- Remove debug-level global config block from Caddyfile
- Change Caddyfile log level from DEBUG to INFO
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 14, 2025

📝 Walkthrough

Walkthrough

Three files were modified to remove or simplify configuration and testing infrastructure. The README.md had an environment variable line removed from deployment instructions. The entire services/db/test-setup.sh file, which contained a 12-step end-to-end test workflow for a Tale DB Docker container setup, was deleted. The services/proxy/Caddyfile was updated to remove the global configuration block with debug and email settings and to change the log level from DEBUG to INFO.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that the database testing functionality provided by test-setup.sh has been relocated to another test suite or framework
  • Confirm that changing the proxy log level from DEBUG to INFO does not suppress critical operational logs needed for monitoring or troubleshooting
  • Check if the ACME_EMAIL removal from README.md is intentional or if it should be documented elsewhere in the deployment guide

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between d1dbc84 and e439319.

📒 Files selected for processing (3)
  • README.md (0 hunks)
  • services/db/test-setup.sh (0 hunks)
  • services/proxy/Caddyfile (1 hunks)
💤 Files with no reviewable changes (2)
  • README.md
  • services/db/test-setup.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (.cursor/rules/workspace_rules.mdc)

Use English only for ALL user-facing content including UI components, labels, buttons, dialogs, forms, toast messages, error messages, success messages, comments, documentation, README files, variable names, function names, and type names

Files:

  • services/proxy/Caddyfile
🔇 Additional comments (2)
services/proxy/Caddyfile (2)

24-24: LGTM! Appropriate log level for production.

Changing from DEBUG to INFO reduces log verbosity and is the standard choice for production environments.


14-19: Verify removal of ACME email configuration is intentional.

The PR removed the ACME email configuration (both from the global config and the README example). While Caddy's automatic certificate renewal will continue to function, ACME providers like Let's Encrypt and ZeroSSL use email addresses to send important notifications about certificate expiration, renewal failures, or account issues.

Please confirm:

  1. Is the removal of ACME email intentional?
  2. Are certificate expiration notifications being monitored through alternative means?

If notifications are desired, consider adding the email back to the ACME issuer configuration:

 tls {
   issuer acme {
     dir {$ACME_CA:https://acme-v02.api.letsencrypt.org/directory}
+    email {$ACME_EMAIL}
     disable_tlsalpn_challenge
   }
 }

Comment @coderabbitai help to get the list of available commands and usage tips.

@larryro larryro merged commit 69abb6b into main Dec 14, 2025
1 check passed
@larryro larryro deleted the code-cleanup branch December 14, 2025 12:53
larryro added a commit that referenced this pull request May 9, 2026
…inding

Five clustered cross-tenant + IDOR P0s flagged by the two-round
data-protection review (round-1 #5, #16, #17, #27, round-2 V1, V7, V8):

P0-1, P0-2: generic admin-trash restore
  `restoreRowToActive` (soft_delete_helpers) added required `organizationId`
  arg + `userMembershipIds` set; refuses cross-org rows with `not_found`
  (no foreign-id existence oracle) and rows whose author is on a custodian
  hold. Added `authorField` to `SOFT_DELETE_RESOURCE_CONFIG` per resource
  type so the cascade fires uniformly across the 14 restorable types.
  `restoreSoftDeletedRow` (governance/restore.ts) threads holds + orgId.

P0-3, P0-4: REST cross-tenant on GET/PATCH/DELETE
  Added optional `callerOrgId` arg to every internal query/mutation
  reachable from REST: threads (`getThreadMetadata`,
  `getThreadMessagesInternal`), customers (get/update/delete), vendors
  (get/update/delete), products (get/update/delete), documents
  (getDocumentByIdRaw, updateDocument, deleteDocumentById), workflow
  executions (getExecution, getRawExecution, listExecutionsCursorInternal,
  getExecutionStepJournalInternal), workflow triggers
  (cancelExecutionInternal, updateScheduleInternal, deleteScheduleInternal,
  deleteWebhookInternal). Each REST handler passes `rc.org.organizationId`.
  Mismatches return null/empty (so REST surfaces 404) — never throw
  cross-tenantly to avoid leaking row existence.

P0-5: rag_search IDOR + thread-binding
  Architectural change: chat uploads bind to their thread at upload time.
  - file_metadata schema: new optional `threadId` + composite index
    `by_organizationId_and_threadId`.
  - file_metadata.saveFileMetadata: accepts threadId; refuses cross-org
    threadId via threadMetadata lookup.
  - chat-input upload hook: passes the current chat threadId.
  - New helpers: `threads/get_thread_ancestor_chain.ts` (action-side walk
    of the delegation chain via existing `getParentThreadId`),
    `agent_tools/rag/helpers/verify_thread_scoped_access.ts` (query that
    same-org-checks each storageId, then if `meta.threadId` is set,
    requires it in the caller's accessible chain).
  - rag_search search-op: when explicit fileIds are supplied, verify
    via the new query (replaces unauthenticated short-circuit).
  - rag_search retrieve-op: replaces commit d7bc3da's stricter
    "agent allow-list" check, which over-strictly blocked legitimate
    chat-upload retrieval (chat uploads never appear in the agent's
    pre-configured `knowledgeFileIds`/`agentTeamId`/`includeOrgKnowledge`
    sets). Same-org + thread-scope is the correct invariant.

Cascade integration:
  - threads/cascade_helpers.ts: deletes bound fileMetadata + storage
    blobs as part of thread hard-delete (Pass B / GDPR erasure path).
    RAG-side fan-out is deferred to commit 4 (P0-13).
  - threads/delete_chat_thread.ts (Pass A trash): cascades soft-delete
    to bound fileMetadata rows (lifecycleStatus='trashed', mirrors
    grace-extension defense).
  - threads/restore_chat_thread.ts: cascades restore to fileMetadata
    rows trashed within ±5s skew of the thread's trash time (avoids
    zombie-restoring files that were independently trashed earlier).

Verified: `bun typecheck` clean; 871 tests across affected areas pass.
Regression tests for the new gates land in commit 6 (test guards) per
the bundle plan.
larryro added a commit that referenced this pull request May 9, 2026
…inding

Five clustered cross-tenant + IDOR P0s flagged by the two-round
data-protection review (round-1 #5, #16, #17, #27, round-2 V1, V7, V8):

P0-1, P0-2: generic admin-trash restore
  `restoreRowToActive` (soft_delete_helpers) added required `organizationId`
  arg + `userMembershipIds` set; refuses cross-org rows with `not_found`
  (no foreign-id existence oracle) and rows whose author is on a custodian
  hold. Added `authorField` to `SOFT_DELETE_RESOURCE_CONFIG` per resource
  type so the cascade fires uniformly across the 14 restorable types.
  `restoreSoftDeletedRow` (governance/restore.ts) threads holds + orgId.

P0-3, P0-4: REST cross-tenant on GET/PATCH/DELETE
  Added optional `callerOrgId` arg to every internal query/mutation
  reachable from REST: threads (`getThreadMetadata`,
  `getThreadMessagesInternal`), customers (get/update/delete), vendors
  (get/update/delete), products (get/update/delete), documents
  (getDocumentByIdRaw, updateDocument, deleteDocumentById), workflow
  executions (getExecution, getRawExecution, listExecutionsCursorInternal,
  getExecutionStepJournalInternal), workflow triggers
  (cancelExecutionInternal, updateScheduleInternal, deleteScheduleInternal,
  deleteWebhookInternal). Each REST handler passes `rc.org.organizationId`.
  Mismatches return null/empty (so REST surfaces 404) — never throw
  cross-tenantly to avoid leaking row existence.

P0-5: rag_search IDOR + thread-binding
  Architectural change: chat uploads bind to their thread at upload time.
  - file_metadata schema: new optional `threadId` + composite index
    `by_organizationId_and_threadId`.
  - file_metadata.saveFileMetadata: accepts threadId; refuses cross-org
    threadId via threadMetadata lookup.
  - chat-input upload hook: passes the current chat threadId.
  - New helpers: `threads/get_thread_ancestor_chain.ts` (action-side walk
    of the delegation chain via existing `getParentThreadId`),
    `agent_tools/rag/helpers/verify_thread_scoped_access.ts` (query that
    same-org-checks each storageId, then if `meta.threadId` is set,
    requires it in the caller's accessible chain).
  - rag_search search-op: when explicit fileIds are supplied, verify
    via the new query (replaces unauthenticated short-circuit).
  - rag_search retrieve-op: replaces commit d7bc3da's stricter
    "agent allow-list" check, which over-strictly blocked legitimate
    chat-upload retrieval (chat uploads never appear in the agent's
    pre-configured `knowledgeFileIds`/`agentTeamId`/`includeOrgKnowledge`
    sets). Same-org + thread-scope is the correct invariant.

Cascade integration:
  - threads/cascade_helpers.ts: deletes bound fileMetadata + storage
    blobs as part of thread hard-delete (Pass B / GDPR erasure path).
    RAG-side fan-out is deferred to commit 4 (P0-13).
  - threads/delete_chat_thread.ts (Pass A trash): cascades soft-delete
    to bound fileMetadata rows (lifecycleStatus='trashed', mirrors
    grace-extension defense).
  - threads/restore_chat_thread.ts: cascades restore to fileMetadata
    rows trashed within ±5s skew of the thread's trash time (avoids
    zombie-restoring files that were independently trashed earlier).

Verified: `bun typecheck` clean; 871 tests across affected areas pass.
Regression tests for the new gates land in commit 6 (test guards) per
the bundle plan.
larryro added a commit that referenced this pull request May 17, 2026
Closes #15, #16, #17, #18, #41 — secret sanitiser + audio-fetch headers.

- `sanitize_secrets.ts` JSON-value class now honours backslash-escaped
  characters: `(?:[^"\\]|\\.)*` instead of `[^"]*`. Previously a value
  containing `\"` (an escaped quote inside the string) terminated the
  redaction at the first inner quote and left the rest of the JSON
  object exposed. Test added that pins the new behaviour.
- `sanitize_secrets.ts` patterns extended with Stripe (`sk_/pk_/rk_` ×
  `live/test`), OpenAI org/project identifiers (`org-…`, `proj_…`),
  Cookie / Set-Cookie header lines, and bare Better Auth session
  tokens. An idempotency check (sanitize ∘ sanitize = sanitize) and a
  100k-char ReDoS guard round out the new coverage.
- `http.ts /api/tts-audio` moves the per-IP rate limit ABOVE the Better
  Auth session lookup. An unauthenticated attacker hammering the route
  was previously forcing a DB session-query per request — mirrors the
  `/storage` route's order.
- `http.ts /api/tts-audio` Cache-Control changes from
  `private, max-age=600` to `private, max-age=0, must-revalidate`. A
  removed member can no longer replay cached audio for up to 10 minutes
  past their removal; the revalidating round-trip re-hits the
  membership gate on every replay.
- `http.ts /api/tts-audio` sets `Accept-Ranges: none` so iOS Safari
  `<audio preload="auto">` range probes don't trigger audible mid-chunk
  restarts.
larryro added a commit that referenced this pull request May 17, 2026
Closes #15, #16, #17, #18, #41 — secret sanitiser + audio-fetch headers.

- `sanitize_secrets.ts` JSON-value class now honours backslash-escaped
  characters: `(?:[^"\\]|\\.)*` instead of `[^"]*`. Previously a value
  containing `\"` (an escaped quote inside the string) terminated the
  redaction at the first inner quote and left the rest of the JSON
  object exposed. Test added that pins the new behaviour.
- `sanitize_secrets.ts` patterns extended with Stripe (`sk_/pk_/rk_` ×
  `live/test`), OpenAI org/project identifiers (`org-…`, `proj_…`),
  Cookie / Set-Cookie header lines, and bare Better Auth session
  tokens. An idempotency check (sanitize ∘ sanitize = sanitize) and a
  100k-char ReDoS guard round out the new coverage.
- `http.ts /api/tts-audio` moves the per-IP rate limit ABOVE the Better
  Auth session lookup. An unauthenticated attacker hammering the route
  was previously forcing a DB session-query per request — mirrors the
  `/storage` route's order.
- `http.ts /api/tts-audio` Cache-Control changes from
  `private, max-age=600` to `private, max-age=0, must-revalidate`. A
  removed member can no longer replay cached audio for up to 10 minutes
  past their removal; the revalidating round-trip re-hits the
  membership gate on every replay.
- `http.ts /api/tts-audio` sets `Accept-Ranges: none` so iOS Safari
  `<audio preload="auto">` range probes don't trigger audible mid-chunk
  restarts.
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.

1 participant