apps/dashboard/lib/utils.ts (2)

34-56: Documentation is comprehensive but needs minor improvements

The documentation is thorough but could be enhanced:

• Return type in documentation mentions "undefined" but code returns "null"
• Example code shows undefined check but should show null check

Apply these documentation improvements:

 * Checks if a workspace has access to a specific feature or beta feature
-* Returns the feature value if access is granted, undefined otherwise
+* Returns the feature value if access is granted, null otherwise
 * Note: Always returns true in development environment
 *
 * @param flagName - The name of the feature to check
 * @param workspace - The workspace to check access for
-* @returns The feature value (boolean | number | string) if access granted, undefined otherwise
+* @returns The feature value (boolean | number | string) if access granted, null otherwise
 *
 * @example
 * ```typescript
 * // Check if workspace has access to logs page
 * if (!hasWorkspaceAccess("logsPage", workspace)) {
 *   return notFound();
 * }
 *
 * // Check if workspace has access to a feature with numeric value
 * const userLimit = hasWorkspaceAccess("userLimit", workspace);
-* if (userLimit === undefined) {
+* if (userLimit === null) {
 *   return notFound();
 * }
 * ```

---

57-82: Consider security implications of development bypass

The implementation looks solid but has some security considerations:

1. Development bypass returns true for all features
2. No logging of bypassed checks in development

Consider these improvements:

1. Add environment variable to explicitly enable bypass mode
2. Add debug logging in development mode
3. Consider allowing selective feature bypass instead of all-or-nothing

Example implementation:

const FEATURE_BYPASS_ENABLED = process.env.FEATURE_BYPASS_ENABLED === 'true';
const BYPASS_FEATURES = process.env.BYPASS_FEATURES?.split(',') || [];

export function hasWorkspaceAccess<T extends keyof ConfigObject>(
  flagName: T,
  workspace: Partial<WorkspaceFeatures>,
): FlagValue<T> | null {
  if (process.env.NODE_ENV === "development" && FEATURE_BYPASS_ENABLED) {
    if (BYPASS_FEATURES.length === 0 || BYPASS_FEATURES.includes(flagName)) {
      console.debug(`[Feature Bypass] Enabling feature: ${flagName}`);
      return true as FlagValue<T>;
    }
  }
  // ... rest of the implementation
}

apps/dashboard/app/(app)/logs/page.tsx (2)

32-34: Consider enhancing error handling and user feedback

The implementation correctly uses hasWorkspaceAccess but could benefit from better error handling:

1. Consider showing a more user-friendly message instead of 404
2. Add logging for debugging purposes in development

Consider this enhancement:

   if (!hasWorkspaceAccess("logsPage", workspace)) {
+    if (process.env.NODE_ENV === "development") {
+      console.debug("[Logs Page] Access denied: logsPage feature not enabled");
+    }
+    return (
+      <div className="text-center p-4">
+        <h2>Logs Page Access Restricted</h2>
+        <p>Please contact your administrator to enable this feature.</p>
+      </div>
+    );
-    return notFound();
   }

---

Line range hint 44-46: Enhance error handling for ClickHouse errors

The error handling for ClickHouse could be more robust:

   if (logs.err) {
-    throw new Error(`Something went wrong when fetching logs from ClickHouse: ${logs.err.message}`);
+    console.error("[ClickHouse] Error fetching logs:", logs.err);
+    return (
+      <div className="text-center p-4">
+        <h2>Unable to Load Logs</h2>
+        <p>There was an error loading the logs. Please try again later.</p>
+        {process.env.NODE_ENV === "development" && (
+          <pre className="mt-4 p-2 bg-gray-100 rounded">
+            {logs.err.message}
+          </pre>
+        )}
+      </div>
+    );
   }

apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)

59-61: Consider extracting the default ratelimit override limit as a constant

The hardcoded value of 5 should be extracted into a named constant to improve maintainability and document its purpose.

+const DEFAULT_RATELIMIT_OVERRIDE_LIMIT = 5;
+
 {Intl.NumberFormat().format(
-  hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) ?? 5,
+  hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) ?? DEFAULT_RATELIMIT_OVERRIDE_LIMIT,
 )}

apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)

Line range hint 51-64: Consider moving database operations to a service layer

The keyAuth size update logic should be moved to a dedicated service layer for better separation of concerns and maintainability. Additionally, consider:

1. Extracting the cache duration (60_000ms) as a named constant
2. Adding a lock mechanism to prevent race conditions during concurrent updates

Example service layer implementation:

// services/keyAuth.ts
const KEY_AUTH_SIZE_CACHE_DURATION = 60_000; // 1 minute

export class KeyAuthService {
  static async updateSizeIfStale(keyAuth: KeyAuth): Promise<void> {
    if (keyAuth.sizeLastUpdatedAt >= Date.now() - KEY_AUTH_SIZE_CACHE_DURATION) {
      return;
    }

    // TODO: Implement distributed locking mechanism
    const res = await db
      .select({ count: sql<string>`count(*)` })
      .from(schema.keys)
      .where(and(
        eq(schema.keys.keyAuthId, keyAuth.id),
        isNull(schema.keys.deletedAt)
      ));

    await db
      .update(schema.keyAuth)
      .set({
        sizeApprox: Number.parseInt(res?.at(0)?.count ?? "0"),
        sizeLastUpdatedAt: Date.now(),
      })
      .where(eq(schema.keyAuth.id, keyAuth.id));
  }
}

apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts (1)

Line range hint 62-68: Extract support email into configuration

The support email address is hardcoded in error messages. Consider extracting it into a configuration value for easier maintenance.

+const SUPPORT_EMAIL = process.env.SUPPORT_EMAIL ?? "[email protected]";
+
 if (!hasWorkspaceAccess("ipWhitelist", api.workspace)) {
   throw new TRPCError({
     code: "FORBIDDEN",
     message:
-      "IP Whitelisting is only available for enterprise plans. Please contact [email protected].",
+      `IP Whitelisting is only available for enterprise plans. Please contact ${SUPPORT_EMAIL}.`,
   });
 }

apps/dashboard/app/(app)/workspace-navigations.tsx (1)

93-93: Consider memoizing feature access checks

The implementation looks good, but there are multiple calls to hasWorkspaceAccess for different features. Consider memoizing these checks to optimize performance.

Consider creating a memoized helper:

const getFeatureAccess = (workspace: Workspace) => {
  const memo = new Map<string, boolean>();
  return (feature: string) => {
    if (!memo.has(feature)) {
      memo.set(feature, hasWorkspaceAccess(feature, workspace));
    }
    return memo.get(feature)!;
  };
};

Then use it in createWorkspaceNavigation:

const checkAccess = getFeatureAccess(workspace);
// ...
hidden: !checkAccess("webhooks"),
// ...

Also applies to: 100-100, 108-108, 122-122

apps/dashboard/app/(app)/audit/[bucket]/page.tsx (1)

94-94: Consider refactoring the retention days logic for better readability

The current nested nullish coalescing with ternary operator makes the logic hard to follow. Consider breaking it down into more explicit conditions.

Here's a suggested refactor:

-    hasWorkspaceAccess("auditLogRetentionDays", workspace) ?? workspace.plan === "free" ? 30 : 90;
+    const customRetentionDays = hasWorkspaceAccess("auditLogRetentionDays", workspace);
+    if (customRetentionDays !== null) {
+      return customRetentionDays;
+    }
+    return workspace.plan === "free" ? 30 : 90;

This makes it clearer that we:

1. First check for custom retention days via feature flag
2. Fall back to plan-based retention days if no custom value is set

Configuration used: CodeRabbit UI
Review profile: CHILL

+  if (!workspace) {
+    return redirect("/auth/sign-in");
+  }
+
   if (!hasWorkspaceAccess("identities", workspace)) {
     children = (
       <OptIn title="Identities" description="Identities are in beta" feature="identities" />
     );
   }

#!/bin/bash
# Search for direct feature access patterns that might need updating
rg -g '!{*.test.*,*.spec.*}' -A 2 'workspace\.features\.' 

# Search for existing hasWorkspaceAccess usage for consistency
rg -g '!{*.test.*,*.spec.*}' -A 2 'hasWorkspaceAccess\('

apps/dashboard/lib/utils.ts (3)

61-63: Consider using a type-safe development mode bypass

The current type assertion true as FlagValue<T> might hide type mismatches when the actual feature value is expected to be a number or string.

Consider this safer alternative:

-  return true as FlagValue<T>;
+  return (typeof workspace?.features?.[flagName] === 'number' ? 1 : true) as FlagValue<T>;

---

34-56: Enhance documentation with return type examples

The documentation is comprehensive but could be more explicit about return types for different feature types.

Add examples for numeric and string feature flags:

 * @example
 * ```typescript
 * // Check if workspace has access to logs page
 * if (!flag("logsPage", workspace)) {
 *   return notFound();
 * }
 *
 * // Check if workspace has access to a feature with numeric value
 * const userLimit = flag("userLimit", workspace);
 * if (userLimit === undefined) {
 *   return notFound();
 * }
+* 
+* // Example with string feature value
+* const tier = flag("subscriptionTier", workspace);
+* if (tier === "premium") {
+*   // Enable premium features
+* }
 * ```

---

69-79: Improve type safety of feature value casting

The type assertions when returning feature values could potentially hide type mismatches.

Consider adding runtime type checking:

-  return betaFeature as FlagValue<T>;
+  const expectedType = typeof workspace.features?.[flagName];
+  if (expectedType !== typeof betaFeature) {
+    console.warn(`Type mismatch for feature ${String(flagName)}: expected ${expectedType}, got ${typeof betaFeature}`);
+  }
+  return betaFeature as FlagValue<T>;

apps/dashboard/app/(app)/workspace-navigations.tsx (1)

59-59: Consider adding type safety for segments parameter

While the workspace type has been improved, the segments parameter could benefit from stronger typing.

Consider adding a union type for valid segments:

-  segments: string[],
+  segments: Array<'apis' | 'ratelimits' | 'authorization' | 'audit' | 'verifications' | 'logs' | 'success' | 'semantic-cache' | 'identities' | 'settings'>,

Configuration used: CodeRabbit UI
Review profile: CHILL

Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.

apps/dashboard/lib/utils.ts (2)

28-32: Consider more domain-specific type names

While the types are well-structured, consider renaming them to be more domain-specific:

• WorkspaceFeatures → WorkspaceFeatureConfig
• ConfigObject → FeatureFlagConfig
• FlagValue → FeatureFlagValue

This would make their purpose more immediately clear to other developers.

---

34-56: Fix return type documentation

The documentation states "undefined otherwise" but the function returns null. Update the documentation to match the implementation:

- * Returns the feature value if access is granted, undefined otherwise
+ * Returns the feature value if access is granted, null otherwise

Also in the example:

- * if (userLimit === undefined) {
+ * if (userLimit === null) {

Configuration used: CodeRabbit UI
Review profile: CHILL

-export function getFlag<T extends keyof ConfigObject>(
+export function flag<T extends keyof ConfigObject>(

apps/dashboard/lib/utils.ts (1)

34-68: Add @throws to JSDoc documentation.

The documentation is comprehensive but should document the error case when no workspace is found.

Add the following to the JSDoc:

 * @returns The feature value (boolean | number | string) based on environment and settings
+* @throws {Error} When no workspace is found in the database
 *
 * @example

Configuration used: CodeRabbit UI
Review profile: CHILL

+function validateFeatureValue<T>(value: unknown, flagName: string): value is T {
+  // Add type-specific validation based on flagName
+  return true;
+}

 const betaFeature = workspace.betaFeatures?.[flagName as keyof Workspace["betaFeatures"]];
-if (betaFeature !== undefined) {
+if (betaFeature !== undefined && validateFeatureValue<FlagValue<TFlagName>>(betaFeature, flagName)) {
   return betaFeature as FlagValue<TFlagName>;
 }

 const feature = workspace.features?.[flagName as keyof Workspace["features"]];
-if (feature !== undefined) {
+if (feature !== undefined && validateFeatureValue<FlagValue<TFlagName>>(feature, flagName)) {
   return feature as FlagValue<TFlagName>;
 }

#!/bin/bash
# Search for Workspace type definition to understand exposed fields
ast-grep --pattern 'type Workspace = {
  $$$
}'

#!/bin/bash
# Search for Workspace type/interface definition with different patterns
ast-grep --pattern 'interface Workspace {
  $$$
}'

# Also search for potential type imports
rg -l "Workspace.*from.*@" 

# Search for Workspace type usage patterns
rg "type.*Workspace" -A 2

#!/bin/bash
# Let's check the actual Workspace type definition in internal/db
cat internal/db/src/types.ts

# Let's also check the schema definition
cat internal/db/src/schema.ts

# Check the actual usage context
cat apps/dashboard/app/\(app\)/workspace-navigations.tsx

#!/bin/bash
# Let's check the schema definition in a different location since it wasn't found
fd schema.ts

# And then read the content of any relevant schema files
cat internal/db/schema.ts