Skip to content

feat: add apphosting secrets revokeaccess command#10669

Open
7hokerz wants to merge 2 commits into
firebase:mainfrom
7hokerz:7hokerz/add-revokeaccess-command
Open

feat: add apphosting secrets revokeaccess command#10669
7hokerz wants to merge 2 commits into
firebase:mainfrom
7hokerz:7hokerz/add-revokeaccess-command

Conversation

@7hokerz

@7hokerz 7hokerz commented Jun 17, 2026

Copy link
Copy Markdown

Description

Adds apphosting:secrets:revokeaccess, the inverse command for apphosting:secrets:grantaccess.

The command supports revoking Secret Manager access from either:

  • App Hosting backend service accounts via --backend
  • User or group emails via --emails

For backend revocation, this removes the selected backend service accounts from:

  • roles/secretmanager.secretAccessor
  • roles/secretmanager.viewer

(It intentionally preserves roles/secretmanager.secretVersionManager for the App Hosting service agent because that permission is project/App Hosting service-agent scoped rather than backend-specific.)
(However, it may be inaccurate, so It would be great if a reviewer could review this.)

Scenarios Tested

  • Ran npx mocha src/apphosting/secrets/index.spec.ts
  • Ran npm run test:compile
  • Ran npm run lint:changed-files
  • Manual test with node lib/bin/firebase.js apphosting:secrets:revokeaccess ... --debug

Sample Commands

npx firebase apphosting:secrets:revokeaccess

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the apphosting:secrets:revokeaccess command, allowing users to revoke permissions on specified secrets for service accounts, users, or groups. It implements the underlying helper functions revokeSecretAccess and revokeEmailsSecretAccess along with their unit tests. The review feedback suggests robustly handling potential whitespace in comma-separated inputs (secret names and emails) by trimming them, and optimizing performance by parallelizing sequential asynchronous operations (such as secret existence checks and IAM binding revocations) using Promise.all.

Comment thread src/commands/apphosting-secrets-revokeaccess.ts
Comment thread src/commands/apphosting-secrets-revokeaccess.ts
Comment thread src/apphosting/secrets/index.ts
@7hokerz

7hokerz commented Jun 17, 2026

Copy link
Copy Markdown
Author

@aalej @joehan , Could you please take a look at this when you have a moment?

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@7e8582d). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/apphosting/secrets/index.ts 90.24% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10669   +/-   ##
=======================================
  Coverage        ?   57.76%           
=======================================
  Files           ?      608           
  Lines           ?    39154           
  Branches        ?     7845           
=======================================
  Hits            ?    22619           
  Misses          ?    14726           
  Partials        ?     1809           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

4 participants