Skip to content

fix(proxy): restore pusher email from the token cache - #1713

Open
sankalpsthakur wants to merge 1 commit into
finos:mainfrom
sankalpsthakur:fix/1400-token-cache-email
Open

fix(proxy): restore pusher email from the token cache#1713
sankalpsthakur wants to merge 1 commit into
finos:mainfrom
sankalpsthakur:fix/1400-token-cache-email

Conversation

@sankalpsthakur

@sankalpsthakur sankalpsthakur commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

checkUserPushPermission looks up the pusher by action.userEmail. After #1604, resolveUserFromToken maps a GitHub PAT to a GitProxy user on a cold cache, but the token cache only stored the username. On a cache hit the username was overwritten and the last-commit committer email from parsePush was left in place, so the permission check ran against the wrong user (the remaining path on #1400 that @jescalada reproduced). Tag pushes never ran resolveUserFromToken at all.

  • Cache { username, email } and restore both on hit
  • Run resolveUserFromToken first on the tag-push chain, same as branch pushes

Does not close #1400: GHES / non-github.com providers and unmatched gitAccount are unchanged.

Validation

  • npx vitest run test/processors/resolveUserFromToken.test.ts test/chain.test.ts test/testCheckUserPushPermission.test.ts
  • 58 passed (30 + 25 + 3)

Related to #1400

AI/LLM disclosure

  • AI coding tools (including Grok and/or Codex agent-assisted editing) were used to help draft or modify code and this PR description.
  • I reviewed the complete change, understand the reasoning, and ran the reported local tests before submitting.
  • This submission is original work of authorship under the project CLA / contributor terms; AI output was not pasted unreviewed.

@sankalpsthakur
sankalpsthakur requested a review from a team as a code owner September 3, 2026 22:23
@netlify

netlify Bot commented Sep 3, 2026

Copy link
Copy Markdown

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 23be2ce
🔍 Latest deploy log https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/6a9a17affe560d000813eb8c

@github-actions github-actions Bot added the fix label Sep 3, 2026
@sankalpsthakur
sankalpsthakur force-pushed the fix/1400-token-cache-email branch from 8f35edf to f6f62eb Compare September 3, 2026 22:27
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.06%. Comparing base (4ccae8a) to head (23be2ce).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1713   +/-   ##
=======================================
  Coverage   86.05%   86.06%           
=======================================
  Files         101      101           
  Lines        5571     5574    +3     
  Branches      995      997    +2     
=======================================
+ Hits         4794     4797    +3     
  Misses        526      526           
  Partials      251      251           

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

checkUserPushPermission keys on action.userEmail. The token cache only
stored the username, so a cache hit left the last committer's email in
place and the permission check ran against the wrong user.

Store email with the cached identity, and run resolveUserFromToken on
tag pushes as well as branch pushes.

Related to finos#1400 (cache-hit and tag-chain slices; GHES / unmatched
gitAccount paths are unchanged).

Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
@sankalpsthakur
sankalpsthakur force-pushed the fix/1400-token-cache-email branch from f6f62eb to 23be2ce Compare September 4, 2026 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

checkUserPushPermission is checking the wrong user's permission to push

1 participant