Skip to content

feat(audit-logging): Adds audit logging support #355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jun 18, 2025
Merged

Conversation

msukkari
Copy link
Contributor

@msukkari msukkari commented Jun 17, 2025

Adds an audit logging system as well as new /api/ee/audit GET point to fetch all audit logs. The following events are currently logged:

NOTE: this is an enterprise feature and requires an enterprise license

API Key

api_key.creation_failed
api_key.created
api_key.deletion_failed
api_key.deleted

User

user.creation_failed
user.owner_created
user.jit_provisioning_failed
user.jit_provisioned
user.join_request_creation_failed
user.join_requested
user.join_request_approve_failed
user.join_request_approved
user.join_request_removed
user.invite_failed
user.invites_created
user.invite_accepted

Organization

org.ownership_transfer_failed
org.ownership_transferred

Query Related

query.file_source
query.code_search
query.list_repositories

TODO:

  • Test API
  • Add page in docs
  • Add changelog

Summary by CodeRabbit

  • New Features

    • Introduced audit logging, capturing notable user and system actions for security and compliance.
    • Added API endpoint to fetch audit logs, with access control and structured responses.
    • Audit logs can be enabled via the new environment variable SOURCEBOT_EE_AUDIT_LOGGING_ENABLED.
    • Expanded documentation with a dedicated "Audit Logs" page and environment variable details.
    • Enterprise plans now include audit logging entitlement.
  • Bug Fixes

    • Minor code and documentation style improvements for clarity and consistency.
  • Chores

    • Updated changelog to document the addition of audit logging.

@msukkari msukkari requested a review from brendan-kellam June 17, 2025 03:29
Copy link

coderabbitai bot commented Jun 17, 2025

Walkthrough

This change introduces comprehensive audit logging to the system. It adds an "Audit" table to the database, integrates audit logging into user, organization, and API key actions, and provides a new API endpoint to fetch audit logs. Documentation, environment variables, and entitlement checks are updated to support and describe this new enterprise feature.

Changes

File(s) / Area Change Summary
packages/db/prisma/migrations/.../migration.sql, packages/db/prisma/schema.prisma Added Audit table/model, index, and relation to Org.
packages/web/src/ee/features/audit/types.ts Added zod schemas, types, and IAuditService interface for audit events.
packages/web/src/ee/features/audit/auditService.ts Implemented AuditService for writing audit records to the database.
packages/web/src/ee/features/audit/mockAuditService.ts Added MockAuditService as a stub implementation of IAuditService.
packages/web/src/ee/features/audit/factory.ts Added factory for singleton audit service, choosing real or mock implementation based on environment variable.
packages/web/src/ee/features/audit/actions.ts Added fetchAuditRecords for retrieving audit logs, with authorization and error handling.
packages/web/src/app/api/(server)/ee/audit/route.ts Added new API route to fetch audit logs, with entitlement and environment checks.
packages/web/src/actions.ts, packages/web/src/lib/authUtils.ts Integrated audit logging into user, org, and API key actions; updated callbacks to include apiKeyHash.
packages/web/src/auth.ts Added audit logging for user sign-in and sign-out events.
packages/web/src/features/search/searchApi.ts, fileSourceApi.ts, listReposApi.ts Added audit logging for search and repository query operations. Updated callback signatures for apiKeyHash.
packages/web/src/env.mjs Added SOURCEBOT_EE_AUDIT_LOGGING_ENABLED environment variable.
packages/web/src/initialize.ts Added entitlement validation and runtime checks for audit logging and public access.
packages/shared/src/entitlements.ts Added "audit" entitlement to enterprise plans.
CHANGELOG.md Documented audit logging feature in changelog.
docs/docs.json, docs/docs/configuration/audit-logs.mdx Added navigation and documentation page for audit logs.
docs/docs/configuration/environment-variables.mdx Documented new environment variable for audit logging.
docs/docs/configuration/structured-logging.mdx Updated title capitalization and sidebar title.
docs/docs/deployment-guide.mdx Enhanced code block metadata for clarity.
packages/web/src/ee/features/sso/sso.tsx Removed an extraneous blank line.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant API
    participant AuditService
    participant DB

    User->>API: Perform action (e.g., create API key, sign in)
    API->>AuditService: createAudit(event details)
    AuditService->>DB: Insert audit record
    DB-->>AuditService: Confirmation
    AuditService-->>API: Audit result
    API-->>User: Action response
Loading
sequenceDiagram
    participant Client
    participant API
    participant Entitlements
    participant DB

    Client->>API: GET /api/ee/audit (with headers)
    API->>Entitlements: Check 'audit' entitlement & env variable
    alt Entitlement or env missing
        API-->>Client: Error response (403/404)
    else
        API->>DB: Query audit records for org
        DB-->>API: Audit records
        API-->>Client: Return audit logs JSON
    end
Loading

Suggested reviewers

  • brendan-kellam

Poem

In the warren, logs now grow—
Every hop and every show,
Audits track each bunny feat,
From sign-in paws to search so neat.
With ears alert and whiskers keen,
Compliance shines where logs are seen!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🔭 Outside diff range comments (2)
packages/web/src/auth.ts (1)

103-110: Missing await on onCreateUser leads to race conditions

onCreateUser({ user: authJsUser }); is asynchronous, but the promise isn’t awaited.
A failure (including audit-db write) will be unhandled and the credential login flow continues in an inconsistent state.

- onCreateUser({ user: authJsUser });
+ await onCreateUser({ user: authJsUser });
packages/web/src/actions.ts (1)

466-514: Don’t let audit-logging failures abort business logic

await auditService.createAudit(...) is awaited directly.
If the audit table is unavailable the primary operation (API-key creation) will reject even though it succeeded in prisma. That’s the opposite of what an audit trail should do.

Wrap each call:

try {
  await auditService.createAudit({...});
} catch (err) {
  logger.warn('audit failure', err);
}

or have auditService swallow its own errors.
Apply the same pattern to every createAudit invocation in this file.

🧹 Nitpick comments (14)
packages/web/src/ee/features/audit/mockAuditService.ts (1)

4-7: Consider logging (or at least counting) skipped audits

Returning null is expected for the mock, but completely ignoring the event may hide integration mistakes when running without the audit entitlement. A lightweight console.debug or an internal counter could help surface misuse during development/tests without adding real persistence.

   async createAudit(_event: Omit<AuditEvent, 'sourcebotVersion'>): Promise<Audit | null> {
-    return null;
+    // Intentionally no-op; emit a debug entry so devs know it ran.
+    if (process.env.NODE_ENV !== 'production') {
+      // eslint-disable-next-line no-console
+      console.debug('[MockAuditService] audit skipped', _event.action);
+    }
+    return null;
   }
packages/web/src/features/search/listReposApi.ts (1)

50-63: Possible oversize metadata

Joining all repository names into a single message string may exceed column limits (commonly 255/1024 chars). Consider truncating or storing as JSON array.

packages/web/src/ee/features/audit/factory.ts (1)

14-16: Micro-optimisation: reuse mock instance

When the ‘audit’ entitlement is absent, every getAuditService() call allocates a new MockAuditService. Not critical, but we can mirror the enterprise singleton for parity & minimal GC churn.

-let enterpriseService: IAuditService | null = null;
+let enterpriseService: IAuditService | null = null;
+let mockService: IAuditService | null = null;
@@
   }
-  return new MockAuditService();
+  if (!mockService) {
+    mockService = new MockAuditService();
+  }
+  return mockService;
packages/web/src/features/search/fileSourceApi.ts (2)

56-57: Consider using the raw identifiers instead of the escaped values in target.id.

Storing escapedRepository / escapedFileName means the audit record contains regex-escaped strings rather than the human-readable repo/file. Consumers of the audit log will need to reverse the escaping. Prefer the original inputs unless there is a strict requirement otherwise.


10-16: Lazy-load the audit service to avoid unnecessary EE work in OSS builds.

Importing the enterprise factory and resolving the service at module scope executes on every Cold-Start, even when the current deployment/plan has no audit entitlement. Moving the resolution inside the handler (or memoising per-request) saves bytes for the common path.

Low priority, but worth considering.

packages/web/src/features/entitlements/constants.ts (1)

13-21: Minor: keep entitlement list alphabetically sorted for merge hygiene.

New "audit" entitlement is appended after "code-nav", breaking the existing alphabetical order.
While harmless, keeping the array ordered reduces merge conflicts.

packages/web/src/app/api/(server)/ee/audit/route.ts (1)

14-20: Header name is case-sensitive in many runtimes – document or normalise.

request.headers.get("X-Org-Domain") will fail if the client sends x-org-domain.
Consider normalising via toLowerCase() or documenting the exact casing requirement.

packages/web/src/lib/authUtils.ts (1)

56-73: Repeated literal strings make maintenance error-prone

"user.creation_failed" is hard-coded here while similar strings are scattered elsewhere. Consider extracting audit action constants (e.g. AUDIT_ACTIONS.USER_CREATION_FAILED) to avoid typos and ease future refactors.

packages/web/src/auth.ts (1)

22-23: Module-level singleton may ignore entitlement changes

const auditService = getAuditService(); is evaluated once at import time.
If entitlements are tenant / request scoped (e.g. free vs. enterprise), switching plans at runtime will keep serving the stale instance. Either:

  1. Invoke getAuditService() inside each handler, or
  2. Make getAuditService internally resilient to entitlement changes.
packages/db/prisma/schema.prisma (2)

175-176: Cascading delete of audits may hamper forensics

audits Audit[] with onDelete: Cascade means deleting an org will erase its entire audit trail – defeating one purpose of audits. Consider retaining audits (e.g. Restrict or archival table) instead.


232-248: emails should be an array, not a single string

In AuditMetadata, emails is defined as String in both the DB (Json) and Zod schema, yet semantically represents multiple addresses (see plural name). Change to string[] to avoid delimiting hacks.

-  emails: z.string().optional(),
+  emails: z.array(z.string()).optional(),

Update DB consumers accordingly.

packages/web/src/ee/features/audit/types.ts (2)

16-22: Enum names use snake_case, conflicting with project style

Keys like api_key diverge from camelCase used elsewhere (actorId, sourcebotVersion).
Aligning casing improves DX:

-    api_key: z.string().optional(),
+    apiKey: z.string().optional(),

(Adjust callers & DB payloads.)


34-36: Return type of createAudit ignores nullability root cause

Why would createAudit ever return null rather than throw on failure?
If the intent is “best effort”, prefer returning the persisted Audit object or no value (void) to avoid ambiguous null checks downstream.

packages/web/src/actions.ts (1)

974-992: Duplicate failAuditCallback helpers – extract a utility

Very similar inline helpers are declared here and in several other functions (redeemInvite, transferOwnership, approveAccountRequest, …).
Factor them into a small utility (e.g. logAuditFailure(actorId, action, orgId, target, message, metadata?)) to:

• remove repetition
• enforce consistent field population
• reduce future maintenance overhead

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0caa5a and 559633c.

📒 Files selected for processing (16)
  • packages/db/prisma/migrations/20250617031335_add_audit_table/migration.sql (1 hunks)
  • packages/db/prisma/schema.prisma (2 hunks)
  • packages/web/src/actions.ts (23 hunks)
  • packages/web/src/app/api/(server)/ee/audit/route.ts (1 hunks)
  • packages/web/src/auth.ts (2 hunks)
  • packages/web/src/ee/features/audit/auditService.ts (1 hunks)
  • packages/web/src/ee/features/audit/factory.ts (1 hunks)
  • packages/web/src/ee/features/audit/mockAuditService.ts (1 hunks)
  • packages/web/src/ee/features/audit/types.ts (1 hunks)
  • packages/web/src/ee/features/audit/utils.ts (1 hunks)
  • packages/web/src/ee/features/sso/sso.tsx (1 hunks)
  • packages/web/src/features/entitlements/constants.ts (1 hunks)
  • packages/web/src/features/search/fileSourceApi.ts (2 hunks)
  • packages/web/src/features/search/listReposApi.ts (2 hunks)
  • packages/web/src/features/search/searchApi.ts (3 hunks)
  • packages/web/src/lib/authUtils.ts (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
packages/web/src/ee/features/audit/factory.ts (4)
packages/web/src/ee/features/audit/types.ts (1)
  • IAuditService (34-36)
packages/web/src/features/entitlements/server.ts (1)
  • hasEntitlement (91-94)
packages/web/src/ee/features/audit/auditService.ts (1)
  • AuditService (5-23)
packages/web/src/ee/features/audit/mockAuditService.ts (1)
  • MockAuditService (4-8)
packages/web/src/features/search/listReposApi.ts (1)
packages/web/src/ee/features/audit/factory.ts (1)
  • getAuditService (8-16)
packages/web/src/lib/authUtils.ts (6)
packages/web/src/ee/features/audit/factory.ts (1)
  • getAuditService (8-16)
packages/web/src/lib/constants.ts (2)
  • SINGLE_TENANT_ORG_ID (31-31)
  • SINGLE_TENANT_ORG_DOMAIN (32-32)
packages/web/src/features/entitlements/server.ts (1)
  • hasEntitlement (91-94)
packages/web/src/ee/features/sso/sso.tsx (1)
  • handleJITProvisioning (177-250)
packages/web/src/lib/serviceError.ts (1)
  • ServiceErrorException (16-20)
packages/web/src/actions.ts (1)
  • createAccountRequest (1673-1781)
packages/web/src/ee/features/audit/mockAuditService.ts (1)
packages/web/src/ee/features/audit/types.ts (2)
  • IAuditService (34-36)
  • AuditEvent (32-32)
packages/web/src/ee/features/audit/auditService.ts (1)
packages/web/src/ee/features/audit/types.ts (2)
  • IAuditService (34-36)
  • AuditEvent (32-32)
packages/web/src/app/api/(server)/ee/audit/route.ts (2)
packages/web/src/lib/serviceError.ts (1)
  • serviceErrorResponse (22-30)
packages/web/src/ee/features/audit/utils.ts (1)
  • fetchAuditRecords (8-31)
packages/web/src/ee/features/audit/utils.ts (2)
packages/web/src/actions.ts (3)
  • sew (55-63)
  • withAuth (65-118)
  • withOrgMembership (149-197)
packages/web/src/lib/serviceError.ts (1)
  • serviceErrorResponse (22-30)
packages/web/src/features/search/searchApi.ts (1)
packages/web/src/ee/features/audit/factory.ts (1)
  • getAuditService (8-16)
packages/web/src/auth.ts (2)
packages/web/src/ee/features/audit/factory.ts (1)
  • getAuditService (8-16)
packages/web/src/lib/constants.ts (1)
  • SINGLE_TENANT_ORG_ID (31-31)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (7)
packages/web/src/ee/features/sso/sso.tsx (1)

249-250: Extraneous blank line removal is fine

The removal of the superfluous newline keeps the file tidy; no further action needed.

packages/web/src/features/search/listReposApi.ts (1)

48-66: ```shell
#!/bin/bash
set -e

Show the audit types definition

sed -n '1,200p' packages/web/src/ee/features/audit/types.ts


</details>
<details>
<summary>packages/web/src/features/search/searchApi.ts (1)</summary>

`13-16`: **Static initialisation is fine**  

Importing and initialising the audit service at module scope keeps it singleton per worker – saves repeated entitlement checks.

</details>
<details>
<summary>packages/web/src/app/api/(server)/ee/audit/route.ts (1)</summary>

`22-26`: **Route assumes `fetchAuditRecords` returns raw data – breaks when it returns a `Response`.**

Due to the mismatch noted in `utils.ts`, `result` may already be a `Response`.  
Add an explicit check or refactor `fetchAuditRecords` to always return a `ServiceError` / data, never a `Response`.

```diff
-  if (isServiceError(result)) {
-    return serviceErrorResponse(result);
-  }
-  return Response.json(result);
+  if (isServiceError(result)) {
+    return serviceErrorResponse(result);
+  }
+
+  // result might be a Response in edge-cases; forward it untouched
+  if (result instanceof Response) {
+    return result;
+  }
+
+  return Response.json(result);
packages/web/src/lib/authUtils.ts (1)

121-140: Metadata leak risk

res.message may contain PII or low-level error details. Ensure the message is safe to persist long-term and expose through audit APIs. Scrub or whitelist fields if needed.

packages/db/prisma/migrations/20250617031335_add_audit_table/migration.sql (1)

10-12: sourcebotVersion is NOT NULL but default is missing

All insertions must now populate sourcebotVersion. Verify that AuditService sets it explicitly; otherwise inserts will fail at runtime.

packages/web/src/actions.ts (1)

532-546: ```shell
#!/bin/bash
set -e
rg -n 'type: "org"' -C2 packages/web/src/actions.ts


</details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@msukkari msukkari changed the title [WIP] Audit Logs feat(audit-logging): Adds audit logging support Jun 17, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/web/src/env.mjs (1)

87-88: Expose new EE flag to docs / infra scripts

SOURCEBOT_EE_AUDIT_LOGGING_ENABLED is now part of the schema but there is no accompanying change in the README / deployment manifests. Please double-check that:

  1. examples/.env (or equivalent) includes the new key with the default value.
  2. Helm / Terraform / Dockerfiles that template env vars pass it through.

Small gaps here tend to break CI when a new variable is suddenly required.

packages/web/src/initialize.ts (1)

222-224: Startup validation placed after I/O heavy pruning

validateEntitlements() is invoked only after the old-guest-user pruning (which may hit the DB).
Consider moving the entitlement/flag sanity checks to the very top of initSingleTenancy() so the process fails fast before touching external systems.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 559633c and 9c97889.

📒 Files selected for processing (4)
  • packages/web/src/app/api/(server)/ee/audit/route.ts (1 hunks)
  • packages/web/src/ee/features/audit/factory.ts (1 hunks)
  • packages/web/src/env.mjs (1 hunks)
  • packages/web/src/initialize.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/web/src/ee/features/audit/factory.ts
🔇 Additional comments (2)
packages/web/src/initialize.ts (1)

152-157: Mutual-exclusion check runs only when the licence allows public access

The early-exit guard is wrapped by if (hasPublicAccessEntitlement) which means:

  • If a customer does not own the public-access entitlement but still sets enablePublicAccess: true in the declarative config and turns on audit logging, the code will exit earlier at L148 – good.
  • If a customer does own the public-access entitlement, we forbid the combination – also good.

Just flagging this subtle path so QA can add a regression test for it.

packages/web/src/app/api/(server)/ee/audit/route.ts (1)

41-45: Minor: propagate service-error status to HTTP response

serviceErrorResponse(result) returns a Response, whereas isServiceError(result) is checked first. Good pattern, but if fetchAuditRecords() returns a non-ServiceError object that contains partial failures, they’ll be silently swallowed. Ensure fetchAuditRecords never returns “partial OK” objects or expose a dedicated type.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
docs/docs/configuration/audit-logs.mdx (4)

1-4: Consistent title casing
The frontmatter uses sidebarTitle: Audit logs (lowercase "logs"), while the page title is Audit Logs. For consistency, update the sidebar title to use title case.

Example diff:

--- a/docs/docs/configuration/audit-logs.mdx
+++ b/docs/docs/configuration/audit-logs.mdx
@@
-sidebarTitle: Audit logs
+sidebarTitle: Audit Logs

10-14: Reduce redundant phrasing
The phrase "Audit logs" is repeated immediately after the heading. Consider replacing the second occurrence with "These logs" to improve readability.

Example adjustment:

- Audit logs are a collection of notable events performed by users within a Sourcebot deployment.
+ These logs are a collection of notable events performed by users within a Sourcebot deployment.

18-25: Streamline wording and clarify placeholder

  1. Remove the extra "of" in "fetch all of the audit logs" for conciseness.
  2. The placeholder ~ in X-Org-Domain: ~ is unclear—use a descriptive placeholder like <ORG_DOMAIN>.

Example diff:

@@ Fetch audit logs
- Audit logs are stored in the [postgres database](/docs/overview#architecture) connected to Sourcebot. To fetch all of the audit logs, you can use the following API:
+ Audit logs are stored in the [postgres database](/docs/overview#architecture) connected to Sourcebot. To fetch all audit logs, you can use the following API:
@@ curl --request GET
-  --header 'X-Org-Domain: ~' \
+  --header 'X-Org-Domain: <ORG_DOMAIN>' \

27-107: Condense repetitive example entries
The JSON example response is comprehensive but repeats a similar event structure multiple times. Consider truncating or collapsing entries with an ellipsis to highlight different event types without overwhelming the reader.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 185fcfa and 47adbb4.

📒 Files selected for processing (6)
  • CHANGELOG.md (1 hunks)
  • docs/docs.json (1 hunks)
  • docs/docs/configuration/audit-logs.mdx (1 hunks)
  • docs/docs/configuration/environment-variables.mdx (1 hunks)
  • docs/docs/configuration/structured-logging.mdx (1 hunks)
  • docs/docs/deployment-guide.mdx (2 hunks)
✅ Files skipped from review due to trivial changes (4)
  • docs/docs.json
  • docs/docs/configuration/structured-logging.mdx
  • docs/docs/deployment-guide.mdx
  • docs/docs/configuration/environment-variables.mdx
🧰 Additional context used
🪛 LanguageTool
docs/docs/configuration/audit-logs.mdx

[grammar] ~15-~15: This phrase is duplicated. You should probably use “Audit Logs” only once.
Context: ...your Sourcebot deployment. ## Enabling Audit Logs Audit logs must be explicitly enabled by setting t...

(PHRASE_REPETITION)


[grammar] ~18-~18: This phrase is duplicated. You should probably use “Audit Logs” only once.
Context: ...nment-variables) to true ## Fetching Audit Logs Audit logs are stored in the [postgres database](/...

(PHRASE_REPETITION)


[style] ~19-~19: Consider removing “of” to be more concise
Context: ...cture) connected to Sourcebot. To fetch all of the audit logs, you can use the following A...

(ALL_OF_THE)

CHANGELOG.md

[duplication] ~10-~10: Possible typo: you repeated a word.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Added - Added audit logging. [#355](https://github.co...

(ENGLISH_WORD_REPEAT_RULE)

🔇 Additional comments (2)
CHANGELOG.md (1)

10-11: Changelog entry for audit logging is correctly added
The new "Added" subsection under "Unreleased" captures the audit logging feature with the proper PR reference.

docs/docs/configuration/audit-logs.mdx (1)

109-181: Schema aligns with example response
The JSON schema correctly reflects required properties, types, and enum values matching the example. No further changes needed here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/web/src/actions.ts (1)

39-47: Audit service still eagerly instantiated – comment repeated from previous review
getAuditService() is invoked at module load (line 46). If it throws (mis-configuration, missing entitlement, etc.) every import of actions.ts crashes the app. Please defer instantiation or wrap it in a lazy accessor as suggested earlier.

🧹 Nitpick comments (1)
packages/web/src/actions.ts (1)

970-987: Deduplicate the repeated failAuditCallback helpers

Four nearly identical inline helpers (failAuditCallback) are declared for invites, invite-accept, ownership transfer, and join-request approval. This bloats the file and risks divergence.

Extract a small utility, e.g.:

async function logAuditFailure(params: {
  action: string;
  actorId: string;
  orgId: number;
  target: { id: string; type: string; };
  message: string;
}) {
  try {
    await auditService.createAudit({
      action: params.action,
      actor: { id: params.actorId, type: "user" },
      target: params.target,
      orgId: params.orgId,
      metadata: { message: params.message }
    });
  } catch (err) {
    logger.warn(`Audit log failed (${params.action})`, err);
  }
}

Then call await logAuditFailure({ action: "user.invite_failed", ... }).

Also applies to: 1216-1231, 1385-1401, 1773-1789

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47adbb4 and 4aa5923.

📒 Files selected for processing (8)
  • packages/web/src/actions.ts (26 hunks)
  • packages/web/src/app/api/(server)/ee/audit/route.ts (1 hunks)
  • packages/web/src/ee/features/audit/auditService.ts (1 hunks)
  • packages/web/src/ee/features/audit/types.ts (1 hunks)
  • packages/web/src/ee/features/audit/utils.ts (1 hunks)
  • packages/web/src/features/search/fileSourceApi.ts (2 hunks)
  • packages/web/src/features/search/listReposApi.ts (2 hunks)
  • packages/web/src/features/search/searchApi.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/web/src/ee/features/audit/auditService.ts
  • packages/web/src/ee/features/audit/utils.ts
  • packages/web/src/app/api/(server)/ee/audit/route.ts
  • packages/web/src/features/search/fileSourceApi.ts
  • packages/web/src/features/search/listReposApi.ts
  • packages/web/src/features/search/searchApi.ts
  • packages/web/src/ee/features/audit/types.ts

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
CHANGELOG.md (1)

11-11: Nitpick: avoid verb duplication
The bullet "- Added audit logging." mirrors the heading "Added". Consider rephrasing to "- Audit logging support" or "- Audit logging added" for clarity and brevity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4aa5923 and 51606f2.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • packages/web/src/ee/features/audit/auditService.ts (1 hunks)
  • packages/web/src/features/search/searchApi.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/web/src/ee/features/audit/auditService.ts
  • packages/web/src/features/search/searchApi.ts
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[duplication] ~10-~10: Possible typo: you repeated a word.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Added - Added audit logging. [#355](https://github.co...

(ENGLISH_WORD_REPEAT_RULE)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (1)
CHANGELOG.md (1)

10-11: Approve new audit logging changelog entry
The Unreleased → Added section correctly documents the audit logging feature and links to PR #355.

brendan-kellam
brendan-kellam previously approved these changes Jun 18, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
docs/docs/configuration/audit-logs.mdx (1)

109-137: Excellent comprehensive coverage of audit action types.

This table addresses the previous review comment about including the list of events and provides clear documentation of all supported audit actions with their actor and target types.

🧹 Nitpick comments (4)
packages/web/src/ee/features/audit/actions.ts (1)

42-46: Consider including more context in error logging.

The error logging could benefit from additional context such as the organization domain and user ID to aid in debugging.

-        logger.error('Error fetching audit logs', { error });
+        logger.error('Error fetching audit logs', { 
+          error, 
+          domain, 
+          userId,
+          orgId: org.id 
+        });
docs/docs/configuration/audit-logs.mdx (3)

15-15: Fix header duplication for better readability.

The section headers repeat "Audit Logs"/"Audit logs" which creates redundancy as flagged by the linter.

-## Enabling Audit Logs
+## Enabling
-## Fetching Audit Logs
+## Fetching

Also applies to: 18-18


19-19: Simplify "all of the" for conciseness.

-To fetch all of the audit logs, you can use the following API:
+To fetch all audit logs, you can use the following API:

21-25: Consider adding authentication context to the API example.

The curl example shows both headers but doesn't clearly explain when the API key is required vs optional, or what permissions are needed.

 ```bash icon="terminal" Fetch audit logs
 curl --request GET '$SOURCEBOT_URL/api/ee/audit' \
   --header 'X-Org-Domain: ~' \
   --header 'X-Sourcebot-Api-Key: $SOURCEBOT_OWNER_API_KEY'

+Note: Requires organization OWNER role. The API key header is optional if you're already authenticated, but required for programmatic access.


</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**


<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 51606f2dd122b94b2eb1d65d720f4e5dab1f7a8f and c9876e935b50d5cb5a7adbfb73a1d8a66dd8b3a1.

</details>

<details>
<summary>📒 Files selected for processing (3)</summary>

* `docs/docs/configuration/audit-logs.mdx` (1 hunks)
* `packages/web/src/app/api/(server)/ee/audit/route.ts` (1 hunks)
* `packages/web/src/ee/features/audit/actions.ts` (1 hunks)

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary>

* packages/web/src/app/api/(server)/ee/audit/route.ts

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>docs/docs/configuration/audit-logs.mdx</summary>

[grammar] ~15-~15: This phrase is duplicated. You should probably use “Audit Logs” only once.
Context: ...your Sourcebot deployment.  ## Enabling Audit Logs Audit logs must be explicitly enabled by setting t...

(PHRASE_REPETITION)

---

[grammar] ~18-~18: This phrase is duplicated. You should probably use “Audit Logs” only once.
Context: ...nment-variables) to `true`  ## Fetching Audit Logs Audit logs are stored in the [postgres database](/...

(PHRASE_REPETITION)

---

[style] ~19-~19: Consider removing “of” to be more concise
Context: ...cture) connected to Sourcebot. To fetch all of the audit logs, you can use the following A...

(ALL_OF_THE)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (3)</summary>

* GitHub Check: build
* GitHub Check: build
* GitHub Check: build

</details>

<details>
<summary>🔇 Additional comments (2)</summary><blockquote>

<details>
<summary>packages/web/src/ee/features/audit/actions.ts (2)</summary>

`26-37`: **LGTM! Audit logging of fetch operations is appropriate for compliance.**

Creating an audit log entry when fetching audit logs is the correct approach for security and compliance requirements. This ensures there's a trail of who accessed the audit logs and when.

---

`48-48`: ```shell
#!/bin/bash
# Extract the definition of `sew` from the main actions file
rg -n "export (const|function) sew" -C10 packages/web/src/actions.ts

# See where and how `allowSingleTenantUnauthedAccess` is consumed within `sew`
rg -n "allowSingleTenantUnauthedAccess" -C10 packages/web/src/actions.ts

@msukkari msukkari merged commit 5438298 into main Jun 18, 2025
6 checks passed
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.

2 participants