fix: keep Browser visible with partial denies (#213)

Author: GatewayJ

lib/console-policy-parser.ts

@@ -228,6 +228,54 @@ function matchesRequestedAction(policyActions: string[] | undefined, action: str
   return !!impliedActions && impliedActions.some((implied) => matchAction(policyActions, implied))
 }
 
+function matchesDeniedAction(policyActions: string[] | undefined, action: string): boolean {
+  if (IMPLIED_SCOPES[action]) {
+    // A page scope aggregates independent backend capabilities. Denying one
+    // capability must not hide the page; concrete capability checks still
+    // enforce that deny.
+    return matchAction(policyActions, action) || matchAction(policyActions, CONSOLE_SCOPES.CONSOLE_ADMIN)
+  }
+
+  return matchesRequestedAction(policyActions, action)
+}
+
+function getImpliedCapabilities(scope: string): string[] {
+  const impliedActions = IMPLIED_SCOPES[scope] ?? []
+  const concreteServices = new Set(
+    impliedActions.filter((action) => !action.includes("*")).map((action) => normalizeAction(action).split(":")[0]),
+  )
+
+  // Service wildcards are aliases when concrete actions for that service are
+  // known, but remain capabilities for services represented only by a wildcard.
+  return impliedActions.filter(
+    (action) => !action.includes("*") || !concreteServices.has(normalizeAction(action).split(":")[0]),
+  )
+}
+
+function statementDeniesActionGlobally(statement: ConsoleStatement, action: string): boolean {
+  if (statement.Effect !== "Deny") return false
+
+  const actionDenied =
+    statement.NotAction && statement.NotAction.length > 0
+      ? !matchNotAction(statement.NotAction, action)
+      : matchAction(statement.Action, action)
+
+  if (!actionDenied || isAdminAction(action)) return actionDenied
+  if (statement.NotResource && statement.NotResource.length > 0) return false
+  if (!statement.Resource || statement.Resource.length === 0) return true
+  if (statement.Resource.includes("*")) return true
+
+  return normalizeAction(action).startsWith("s3:") && statement.Resource.includes("arn:aws:s3:::*")
+}
+
+function deniesAllImpliedCapabilities(statements: ConsoleStatement[], scope: string): boolean {
+  const capabilities = getImpliedCapabilities(scope)
+  return (
+    capabilities.length > 0 &&
+    capabilities.every((action) => statements.some((statement) => statementDeniesActionGlobally(statement, action)))
+  )
+}
+
 /**
  * Check if an action is a console scope (starts with "console:" or is "consoleAdmin")
  */
@@ -259,14 +307,14 @@ export function hasConsolePermission(
     }
 
     // If Action is present (or empty array), deny applies to matching actions
-    if (matchesRequestedAction(s.Action, action)) {
+    if (matchesDeniedAction(s.Action, action)) {
       return matchStatementResource(s, resource, action)
     }
 
     return false
   })
 
-  if (denied) return false
+  if (denied || deniesAllImpliedCapabilities(statements, action)) return false
 
   // Check Allow statements
   const allowed = statements.some((s) => {

tests/lib/console-policy-parser.test.ts

@@ -0,0 +1,159 @@
+import test from "node:test"
+import assert from "node:assert/strict"
+
+import { CONSOLE_SCOPES } from "../../lib/console-permissions"
+import { hasConsolePermission, type ConsolePolicy } from "../../lib/console-policy-parser"
+
+const maintainerPolicy: ConsolePolicy = {
+  Version: "2012-10-17",
+  Statement: [
+    {
+      Effect: "Allow",
+      Action: ["s3:*"],
+      Resource: ["arn:aws:s3:::*"],
+    },
+    {
+      Effect: "Deny",
+      Action: ["s3:DeleteObject"],
+      Resource: ["arn:aws:s3:::*"],
+    },
+  ],
+}
+
+test("a denied browser capability does not hide the Browser page", () => {
+  assert.equal(hasConsolePermission(maintainerPolicy, CONSOLE_SCOPES.VIEW_BROWSER), true)
+})
+
+test("a denied browser capability remains unavailable", () => {
+  assert.equal(hasConsolePermission(maintainerPolicy, "s3:DeleteObject", "arn:aws:s3:::test/object.txt"), false)
+})
+
+test("an explicit Browser scope deny hides the Browser page", () => {
+  const policy: ConsolePolicy = {
+    ...maintainerPolicy,
+    Statement: [
+      ...maintainerPolicy.Statement,
+      {
+        Effect: "Deny",
+        Action: [CONSOLE_SCOPES.VIEW_BROWSER],
+        Resource: ["console"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_BROWSER), false)
+})
+
+test("a Console scope wildcard deny hides the Browser page", () => {
+  const policy: ConsolePolicy = {
+    ...maintainerPolicy,
+    Statement: [
+      ...maintainerPolicy.Statement,
+      {
+        Effect: "Deny",
+        Action: ["console:*"],
+        Resource: ["console"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_BROWSER), false)
+})
+
+test("a global S3 wildcard deny hides the Browser page", () => {
+  const policy: ConsolePolicy = {
+    Version: "2012-10-17",
+    Statement: [
+      {
+        Effect: "Allow",
+        Action: ["s3:*"],
+        Resource: ["*"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["s3:*"],
+        Resource: ["arn:aws:s3:::*"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_BROWSER), false)
+})
+
+test("a bucket-scoped S3 wildcard deny does not hide the Browser page", () => {
+  const policy: ConsolePolicy = {
+    Version: "2012-10-17",
+    Statement: [
+      {
+        Effect: "Allow",
+        Action: ["s3:*"],
+        Resource: ["arn:aws:s3:::*"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["s3:*"],
+        Resource: ["arn:aws:s3:::restricted/*"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_BROWSER), true)
+})
+
+test("an admin wildcard deny hides the Users page", () => {
+  const policy: ConsolePolicy = {
+    Version: "2012-10-17",
+    Statement: [
+      {
+        Effect: "Allow",
+        Action: ["admin:*"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["admin:*"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_USERS), false)
+})
+
+test("multiple denies that cover all Users capabilities hide the page", () => {
+  const policy: ConsolePolicy = {
+    Version: "2012-10-17",
+    Statement: [
+      {
+        Effect: "Allow",
+        Action: ["admin:*"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["admin:ListUsers", "admin:CreateUser", "admin:GetUser"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["admin:EnableUser", "admin:DisableUser", "admin:DeleteUser"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_USERS), false)
+})
+
+test("a total deny for one service does not hide a mixed-service page", () => {
+  const policy: ConsolePolicy = {
+    Version: "2012-10-17",
+    Statement: [
+      {
+        Effect: "Allow",
+        Action: ["kms:*"],
+      },
+      {
+        Effect: "Deny",
+        Action: ["admin:*"],
+      },
+    ],
+  }
+
+  assert.equal(hasConsolePermission(policy, CONSOLE_SCOPES.VIEW_SSE_SETTINGS), true)
+})