Use privileged service account for exportData

[pauljohanneskraft]

Use privileged service account for exportData

♻️ Current situation & Problem

The exportData function is still failing and apparently used the privileged service account even before. No idea why it was moved down then... It should hopefully resolve the remaining issue.

Code of Conduct & Contributing Guidelines

By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

• I agree to follow the Code of Conduct and Contributing Guidelines.

[coderabbitai]

📝 Walkthrough

Walkthrough

The exportData function's on-call handler invocation now includes a serviceAccount configuration that passes a privilegedServiceAccount to the wrapper, requiring a corresponding import addition from the helpers module.

Changes

Cohort / File(s)	Summary
Privileged Service Account Configuration functions/src/functions/exportData.ts	Added import of privilegedServiceAccount from ./helpers.js and passed it as the serviceAccount option to the validatedOnCall wrapper function.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Use explicit service accounts for database trigger functions #310: Applies the same privileged service account pattern to multiple on-document-written trigger wrappers, suggesting a coordinated effort to add explicit service account configuration across the codebase.

Suggested reviewers

• PSchmiedmayer

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding a privileged service account to the exportData function.
Description check	✅ Passed	The description is directly related to the changeset, explaining the problem with exportData and the rationale for using the privileged service account.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch elevate-export-patient-data

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

This PR updates the exportData Cloud Function to run under the privileged service account, aligning it with other admin/owner operations that require elevated permissions in this codebase and addressing recent export failures.

Changes:

• Import privilegedServiceAccount into exportData.
• Pass serviceAccount: privilegedServiceAccount to validatedOnCall options for exportData.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

functions/src/functions/exportData.ts (1)

63-65: Document why this function must run as privileged.

Please add a short inline comment here explaining the required permission/capability for exportData so this doesn’t get unintentionally reverted again.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@functions/src/functions/exportData.ts` around lines 63 - 65, Add an inline
comment above the serviceAccount assignment explaining that exportData must run
with privilegedServiceAccount because exportData performs
cross-project/storage/admin operations (reads/writes and creates export buckets
or service logs) that require elevated IAM roles (e.g., storage.admin or
bigquery.admin) and cannot run under the default least-privilege runtime;
reference exportData and privilegedServiceAccount so future reviewers understand
this intent and avoid reverting to a non-privileged account.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@functions/src/functions/exportData.ts`:
- Around line 63-65: Add an inline comment above the serviceAccount assignment
explaining that exportData must run with privilegedServiceAccount because
exportData performs cross-project/storage/admin operations (reads/writes and
creates export buckets or service logs) that require elevated IAM roles (e.g.,
storage.admin or bigquery.admin) and cannot run under the default
least-privilege runtime; reference exportData and privilegedServiceAccount so
future reviewers understand this intent and avoid reverting to a non-privileged
account.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4c3702f7-073a-45b1-86a2-2535c4328e6e

📥 Commits

Reviewing files that changed from the base of the PR and between fcb6956 and c7104a3.

📒 Files selected for processing (1)

• functions/src/functions/exportData.ts

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.72%. Comparing base (fcb6956) to head (c7104a3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #314   +/-   ##
=======================================
  Coverage   82.72%   82.72%           
=======================================
  Files          93       93           
  Lines        3836     3836           
  Branches      976      979    +3     
=======================================
  Hits         3173     3173           
  Misses        656      656           
  Partials        7        7           

Files with missing lines	Coverage Δ
functions/src/functions/exportData.ts	85.19% <ø> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcb6956...c7104a3. Read the comment docs.

🚀 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.