Skip to content

fix(jira-poll): degrade gracefully when project role lookup returns 401 - #6020

Closed
samanthajayasinghe wants to merge 1 commit into
fullsend-ai:mainfrom
samanthajayasinghe:fix/jira-poll-role-401-graceful-degrade
Closed

fix(jira-poll): degrade gracefully when project role lookup returns 401#6020
samanthajayasinghe wants to merge 1 commit into
fullsend-ai:mainfrom
samanthajayasinghe:fix/jira-poll-role-401-graceful-degrade

Conversation

@samanthajayasinghe

Copy link
Copy Markdown

Jira Cloud returns 401 ("You cannot edit the configuration of this project.") when the service account lacks Administer Projects permission, even on a read-only GET /project/{key}/role call. Previously this aborted the entire poll cycle, making --jira-project unusable without project admin access. Now the poller logs a warning and continues with external roles, matching the existing behavior when --jira-project is omitted. Transient errors (5xx) still fail the cycle so checkpoints are not advanced past lost events.

Also maps HTTP 401 to forge.ErrForbidden in the Jira APIError.Unwrap, since Jira Cloud uses 401 for project-level permission denials in addition to the standard 403.

Summary

Related Issue

Changes

Testing

  • make lint passes (stage changes first, then run)
  • Tests added/updated for new or modified logic

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Commits are signed off (DCO) — human and human-directed agent sessions only
  • I wrote this contribution myself and can explain all changes in it

Jira Cloud returns 401 ("You cannot edit the configuration of this
project.") when the service account lacks Administer Projects permission,
even on a read-only GET /project/{key}/role call. Previously this aborted
the entire poll cycle, making --jira-project unusable without project
admin access. Now the poller logs a warning and continues with external
roles, matching the existing behavior when --jira-project is omitted.
Transient errors (5xx) still fail the cycle so checkpoints are not
advanced past lost events.

Also maps HTTP 401 to forge.ErrForbidden in the Jira APIError.Unwrap,
since Jira Cloud uses 401 for project-level permission denials in
addition to the standard 403.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@samanthajayasinghe
samanthajayasinghe requested a review from a team as a code owner August 9, 2026 23:26
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Thank you for your interest in contributing to fullsend, @samanthajayasinghe.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one.

See CONTRIBUTING.md for details.

@github-actions github-actions Bot closed this Aug 9, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Gracefully degrade Jira poller when project role lookup returns 401/403

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Treat Jira role lookup 401/403 as a permission denial and fall back to external roles.
• Preserve fail-fast behavior for non-forbidden role API errors to avoid dropping events.
• Add regression tests for 401 error unwrapping and poll-cycle checkpoint advancement.
Diagram

graph TD
  poller["Jira Poller Run"] --> roleCall["Get project roles"] --> jiraApi["Jira Cloud API"] --> handle{"401/403 -> external"} --> process["Process issues/events"] --> write["Write dispatches"] --> checkpoint["Advance checkpoints"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Handle 401/403 only at the role-membership call site
  • ➕ Avoids globally reclassifying all Jira HTTP 401 responses as forge.ErrForbidden
  • ➕ Keeps room to distinguish “bad credentials” vs “insufficient project permissions” for other endpoints
  • ➖ Requires endpoint-specific logic (status-code checks or error typing) in the poller
  • ➖ Less reusable if other Jira call sites also need the same behavior
2. Introduce a dedicated ErrUnauthorized and map 401 to it
  • ➕ Preserves semantic distinction between authentication failure (401) and authorization failure (403)
  • ➕ Callers can choose whether to degrade on unauthorized vs forbidden
  • ➖ Larger cross-cutting change: new error type and updates across call sites/tests
  • ➖ May not align with Jira Cloud’s nonstandard use of 401 for project permission denials
3. Detect Jira’s project-permission-denied 401 via response message/body
  • ➕ More precise: degrade only for known permission-denial cases while keeping true auth failures obvious
  • ➖ Brittle to Jira message changes/localization
  • ➖ Adds parsing complexity and couples behavior to error text

Recommendation: The PR’s approach is reasonable for this codebase given there is no ErrUnauthorized and Jira Cloud’s documented behavior here is effectively “forbidden.” The main tradeoff is that all Jira 401s now unwrap to forge.ErrForbidden; if future callers need to distinguish invalid credentials from insufficient permissions, consider switching to an endpoint-specific check (or introducing ErrUnauthorized) then.

Files changed (4) +64 / -3

Bug fix (2) +11 / -3
client.goMap Jira 401 responses to forge.ErrForbidden +3/-1

Map Jira 401 responses to forge.ErrForbidden

• Extends APIError.Unwrap() to treat HTTP 401 (Unauthorized) like 403 (Forbidden). This matches Jira Cloud behavior where project-level permission denials can surface as 401 on read-only role endpoints.

internal/forge/jira/client.go

poller.goDegrade to external roles when role lookup is forbidden +8/-2

Degrade to external roles when role lookup is forbidden

• Changes role-membership loading to treat forge.ErrForbidden as a warning rather than a cycle-failing error. Other errors still abort the cycle to avoid advancing checkpoints past potentially missed events.

internal/jirapoll/poller.go

Tests (2) +53 / -0
client_test.goAssert 401 unwraps to forge.ErrForbidden +1/-0

Assert 401 unwraps to forge.ErrForbidden

• Updates the 401 error-response test to verify errors.Is(err, forge.ErrForbidden). This locks in the new unwrapping behavior for Jira API errors.

internal/forge/jira/client_test.go

poller_test.goAdd poll-cycle regression test for forbidden role load +52/-0

Add poll-cycle regression test for forbidden role load

• Adds a test ensuring that a 401/403-equivalent role lookup failure does not abort the cycle and that checkpoints still advance. This complements the existing test that transient failures (e.g., 5xx) must still fail the cycle.

internal/jirapoll/poller_test.go

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. 401 mapped too broadly 🐞 Bug ≡ Correctness
Description
jira.APIError.Unwrap() now maps every HTTP 401 to forge.ErrForbidden, conflating
authentication/configuration failures with authorization denials. Callers that treat ErrForbidden
as non-fatal (e.g., role membership load and status transition resolution) may proceed and advance
checkpoints / drop events instead of failing fast on bad auth.
Code

internal/forge/jira/client.go[R162-165]

+	// Jira Cloud returns 401 for project-level permission denials
+	// (e.g. project role listing) in addition to the standard 403.
+	if e.StatusCode == http.StatusForbidden || e.StatusCode == http.StatusUnauthorized {
		return forge.ErrForbidden
Relevance

●●● Strong

Team has accepted tightening over-broad HTTP→sentinel mappings to avoid misclassification/fail-open
behavior.

PR-#2201
PR-#2595

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The diff changes the Jira error unwrapping to treat 401 as forbidden, and the poller uses
ErrForbidden as a non-fatal signal to continue with external roles; additionally, jira-poll has
other code paths that treat ErrForbidden as a reason to drop events rather than fail, making broad
401->forbidden classification risky.

internal/forge/jira/client.go[158-167]
internal/jirapoll/poller.go[104-125]
internal/jirapoll/discover.go[254-270]
internal/cli/poll.go[200-216]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`APIError.Unwrap()` currently maps HTTP 401 to `forge.ErrForbidden` for all Jira endpoints. This is too broad: 401 can represent invalid credentials/wrong auth scheme, which should not be treated as a mere permission denial because downstream code may intentionally degrade/skip on `ErrForbidden`.

### Issue Context
The poller now explicitly degrades on `errors.Is(err, forge.ErrForbidden)` when loading project role membership; additionally, other jira-poll logic drops some events on `ErrForbidden`.

### Fix approach (recommended)
- Revert `APIError.Unwrap()` to map **only** HTTP 403 -> `forge.ErrForbidden` (keep 404 -> `forge.ErrNotFound`).
- Handle Jira Cloud’s project-role-specific 401 quirk **at the call site**:
 - In `LiveClient.GetProjectRoleMembership`, detect an `*jira.APIError` with `StatusCode==401` (and optionally a known permission-denial message) and wrap it with `forge.ErrForbidden` (e.g., `return nil, fmt.Errorf("%w: %w", forge.ErrForbidden, err)`), so only that endpoint participates in the graceful-degradation path.
- Update/add tests:
 - Add a test that `GetProjectRoleMembership` 401 maps to `forge.ErrForbidden`.
 - Update `TestErrorResponse_401` to assert 401 is **not** `forge.ErrForbidden` for generic endpoints like `/myself` (or rename/split the test to reflect the endpoint-specific behavior).

### Fix Focus Areas
- internal/forge/jira/client.go[158-167]
- internal/forge/jira/client.go[543-548]
- internal/forge/jira/client_test.go[448-465]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Org-specific Jira URL in test 📘 Rule violation ⛨ Security
Description
The new test hardcodes JiraBaseURL as https://acme.atlassian.net and TargetRepo as
acme/platform, which are environment/organization-specific identifiers. This can leak real
tenancy/repo naming and violates the requirement to avoid hardcoded sensitive environment-specific
identifiers in source code/tests.
Code

internal/jirapoll/poller_test.go[R578-581]

+		TargetRepo:  "acme/platform",
+		JiraBaseURL: "https://acme.atlassian.net",
+		JiraProject: "PROJ",
+		OutputPath:  outputPath,
Relevance

●●● Strong

They usually remove environment-specific identifiers from tests/docs; swapping to obvious
placeholders is low-risk.

PR-#761
PR-#5778

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062040 disallows hardcoded sensitive environment-specific identifiers in source
code/tests. The added test introduces a concrete Jira tenant URL and repo name as literals, rather
than clearly fake placeholders.

Rule 1062040: Disallow hardcoded secrets and sensitive environment-specific identifiers in source code
internal/jirapoll/poller_test.go[578-581]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new test hardcodes organization/environment-specific identifiers (`https://acme.atlassian.net`, `acme/platform`). The compliance rule requires avoiding hardcoded sensitive environment-specific identifiers; tests should use clearly fake placeholders.

## Issue Context
This is test code, so using placeholder values (e.g., `https://example.atlassian.net` and `example/repo`) keeps the test realistic without embedding org-specific identifiers.

## Fix Focus Areas
- internal/jirapoll/poller_test.go[578-581]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +578 to +581
TargetRepo: "acme/platform",
JiraBaseURL: "https://acme.atlassian.net",
JiraProject: "PROJ",
OutputPath: outputPath,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Org-specific jira url in test 📘 Rule violation ⛨ Security

The new test hardcodes JiraBaseURL as https://acme.atlassian.net and TargetRepo as
acme/platform, which are environment/organization-specific identifiers. This can leak real
tenancy/repo naming and violates the requirement to avoid hardcoded sensitive environment-specific
identifiers in source code/tests.
Agent Prompt
## Issue description
A new test hardcodes organization/environment-specific identifiers (`https://acme.atlassian.net`, `acme/platform`). The compliance rule requires avoiding hardcoded sensitive environment-specific identifiers; tests should use clearly fake placeholders.

## Issue Context
This is test code, so using placeholder values (e.g., `https://example.atlassian.net` and `example/repo`) keeps the test realistic without embedding org-specific identifiers.

## Fix Focus Areas
- internal/jirapoll/poller_test.go[578-581]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +162 to 165
// Jira Cloud returns 401 for project-level permission denials
// (e.g. project role listing) in addition to the standard 403.
if e.StatusCode == http.StatusForbidden || e.StatusCode == http.StatusUnauthorized {
return forge.ErrForbidden

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. 401 mapped too broadly 🐞 Bug ≡ Correctness

jira.APIError.Unwrap() now maps every HTTP 401 to forge.ErrForbidden, conflating
authentication/configuration failures with authorization denials. Callers that treat ErrForbidden
as non-fatal (e.g., role membership load and status transition resolution) may proceed and advance
checkpoints / drop events instead of failing fast on bad auth.
Agent Prompt
### Issue description
`APIError.Unwrap()` currently maps HTTP 401 to `forge.ErrForbidden` for all Jira endpoints. This is too broad: 401 can represent invalid credentials/wrong auth scheme, which should not be treated as a mere permission denial because downstream code may intentionally degrade/skip on `ErrForbidden`.

### Issue Context
The poller now explicitly degrades on `errors.Is(err, forge.ErrForbidden)` when loading project role membership; additionally, other jira-poll logic drops some events on `ErrForbidden`.

### Fix approach (recommended)
- Revert `APIError.Unwrap()` to map **only** HTTP 403 -> `forge.ErrForbidden` (keep 404 -> `forge.ErrNotFound`).
- Handle Jira Cloud’s project-role-specific 401 quirk **at the call site**:
  - In `LiveClient.GetProjectRoleMembership`, detect an `*jira.APIError` with `StatusCode==401` (and optionally a known permission-denial message) and wrap it with `forge.ErrForbidden` (e.g., `return nil, fmt.Errorf("%w: %w", forge.ErrForbidden, err)`), so only that endpoint participates in the graceful-degradation path.
- Update/add tests:
  - Add a test that `GetProjectRoleMembership` 401 maps to `forge.ErrForbidden`.
  - Update `TestErrorResponse_401` to assert 401 is **not** `forge.ErrForbidden` for generic endpoints like `/myself` (or rename/split the test to reflect the endpoint-specific behavior).

### Fix Focus Areas
- internal/forge/jira/client.go[158-167]
- internal/forge/jira/client.go[543-548]
- internal/forge/jira/client_test.go[448-465]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

1 participant