#213 Feat/oauth jira linear

[Anonymous]

Originally created by: Akarsh-Hegde

Browser OAuth (PKCE) for Jira — additive to the API-token path

Adds a full Authorization Code + PKCE browser login for Jira, wired into the installer so a new user connects without minting an API token. Static JIRA_BASE_URL / JIRA_EMAIL / JIRA_API_TOKEN stays as a fallback; OAuth wins when a token store exists.

OAuth core — new src/intelligence/oauth/

file	role
pkce	S256 verifier / challenge / state
store	daemon-writable ~/.meridian/oauth/jira.json (0600), refresh-aware — rotating refresh tokens are persisted back (unlike static tokens)
flow	generic loopback-redirect engine: browser → code → token exchange → refresh
jira	Atlassian 3LO wiring — login, refresh-before-use, and a JiraReqCtx resolver

• OAuth calls go through api.atlassian.com/ex/jira/{cloudId} with Bearer (cloud-id from accessible-resources); basic auth still hits the site URL — both via one resolve().
• Zero-config: the public client_id is baked into DEFAULT_CLIENT_ID (PKCE has no secret to protect); JIRA_OAUTH_CLIENT_ID overrides it.
• "Configured" = token store exists OR full basic creds present (so non-Jira users get no per-tick auth-fail spam).
• Hardening: the /myself doctor probe works under OAuth via the read:jira-user scope; login() fails loudly if no refresh token comes back (i.e. offline_access not granted); redirect uses 127.0.0.1 (Atlassian's console rejects localhost, and it dodges the localhost→::1 bind trap).

Installer integration

meridian setup (bundle) and ./install.sh (source) now offer "Connect Jira in your browser? [Y/n]" and run the login inline — no separate command, no extra restart. The bundle runs it inline (prebuilt binary present); source defers it to after cargo build, before the daemon starts. API token is the fallback on decline / failure / org-blocked apps. Shared helper: scripts/lib-jira-setup.sh; meridian oauth-login jira also works through the CLI wrapper.

Each user connects to their own Jira site — discovered from whoever signs in. The baked-in client id is only the app identity, not a site.

Maintainer runbook — docs/jira-oauth-app.md

The one-time Atlassian app setup: scopes (read:jira-work write:jira-work read:jira-user + runtime offline_access), the http://127.0.0.1:9123/callback callback, and the Distribution → Distributable toggle that gates all non-Meridiona users. Linked from DEFAULT_CLIENT_ID and SETUP.md, with a pre-GA checklist.

Also in this PR (separate work that landed on the branch)

• feat(tasks) (33ed1c1) — on-demand PM sync, provider filter, and a dashboard sync button (meridian tasks-sync).
• fix(health) (923c315) — tilde expansion in the DB-path check, accurate doctor banner title, doctor DB probe.

Verification — honest status

• ✅ Unit tests: PKCE, token store (save/load/expiry/rotation, 0600 perms), authorize-URL builder, JiraReqCtx URL/auth, config-resolution (store-presence vs basic vs nothing). Bash: lib-jira-setup source-and-call smoke + bash -n on the installers. Full pre-push suite (fmt + clippy + cargo test + UI build + UI tests + security audit) green.
• ⚠️ Not yet confirmed live: the OAuth browser round-trip end-to-end (browser → token store → api.atlassian.com gateway fetch). Earlier "tickets fetched" testing was actually the basic-auth fallback — no token store existed at that moment. The basic-auth path is unchanged and works.

Before this helps external users (no code — operational)

1. Set the Atlassian app Distributable (see the runbook) — otherwise only Meridiona-org users can authorize.
2. Cut a release — the installer integration reaches bundle / npm users only after publish.

Not in this PR

Linear OAuth (same PKCE engine) — held for a follow-up.

🤖 Generated with Claude Code

[Anonymous]

Originally posted by: adityaharishch

Code Review — PR [#213] (OAuth PKCE for Jira)

🔴 Critical Issues (2)

1. jira_update_enabled silently disabled for zero-config OAuth users

• File: src/config.rs:300
• Severity: Critical — breaks core feature (worklog posting)
• Issue: The gate checking whether to enable PM worklog posting checks for env vars JIRA_BASE_URL or JIRA_OAUTH_CLIENT_ID, but not for the OAuth token store file. Users who follow the recommended path (meridian oauth-login jira with zero env vars) will have their OAuth fully configured for task classification but worklog posting will be silently disabled.
• Scenario: User runs meridian oauth-login jira with no env vars → OAuth store created at ~/.meridian/oauth/jira.json → parse_jira() detects it and returns configured → but jira_configured gate on line 300 is false because neither env var is set → jira_update_enabled becomes false → worklogs never post despite OAuth working

• Fix: Add OAuth store check to the gate:

  :::rust
  let jira_configured =
  std::env::var("JIRA_BASE_URL").is_ok()
  || std::env::var("JIRA_OAUTH_CLIENT_ID").is_ok()
  || crate::intelligence::oauth::store::exists("jira"); // ADD THIS

2. Missing 'error' handler on spawn() in tasks sync endpoint

• File: ui/app/api/tasks/sync/route.ts:22-36
• Severity: Critical — unhandled exception crashes API handler
• Issue: The route spawns a subprocess without an 'error' event listener. If the binary doesn't exist (e.g., launchd restricted PATH), Node.js emits an 'error' event. Without a handler, this crashes the Promise chain instead of returning {ok: false}.

• Fix: Add error handler before the timeout handler:

  :::typescript
  child.on('error', (err) => {
  clearTimeout(timer)
  resolve({ ok: false, stdout, stderr: spawn error: ${err.message} })
  })

🟠 High Severity (1)

3. Missing timeouts on HTTP clients in OAuth flow

• Files: src/intelligence/oauth/flow.rs:145, src/intelligence/oauth/jira.rs:88
• Issue: Both post_token() and discover_cloud() create reqwest::Client::new() with no timeout. If Atlassian's servers are slow, ensure_fresh() can block the main daemon poll loop indefinitely.

• Fix: Add timeout to both clients using the pattern from src/health/jira.rs:

  :::rust
  let client = reqwest::Client::builder()
  .timeout(Duration::from_secs(6))
  .build()?;

🟡 Medium Severity (1)

4. Race condition in ensure_fresh() token refresh

• File: src/intelligence/oauth/jira.rs:151-172
• Issue: No locking on the load-refresh-save sequence. If PM sync and health checks call ensure_fresh() concurrently, both load the same refresh_token and POST to Atlassian. Since Atlassian rotates the token on each use, the second request gets a 401.
• Impact: Transient auth-failed warnings during concurrent operation
• Fix: Use tokio::sync::Mutex to serialize refresh calls.

🔵 Low Severity (1)

5. Temporary token file created world-readable

• File: src/intelligence/oauth/store.rs:81
• Issue: Temp file created with default umask (0644) before chmod to 0600. On shared systems, other users can read OAuth tokens during the race window.
• Fix: Set permissions before writing using open() with mode 0o600.

---

Summary: 2 critical bugs (silent feature disable + crash), 1 high (blocking hang), 1 medium (transient auth failure), 1 low (token exposure window).

[Anonymous]

Originally posted by: adityaharishch

Inline: config.rs:300 — CRITICAL

let jira_configured =
    std::env::var("JIRA_BASE_URL").is_ok() || std::env::var("JIRA_OAUTH_CLIENT_ID").is_ok();

This gate only checks env vars, missing OAuth store check. Zero-config OAuth users have:

• parse_jira() → returns Some (detected OAuth store via store::exists("jira"))
• jira_configured → false (no env vars)
• jira_update_enabled → false (silent disable)

Result: Task classification works, worklog posting is silently disabled.

Fix: Add || crate::intelligence::oauth::store::exists("jira") to the condition.

[Anonymous]

Originally posted by: adityaharishch

Inline: ui/app/api/tasks/sync/route.ts:22 — CRITICAL

const child = spawn(bin, ['tasks-sync'], {
  stdio: ['ignore', 'pipe', 'pipe'],
})

let stdout = ''
let stderr = ''
child.stdout?.on('data', (d: Buffer) => { stdout += d.toString() })
child.stderr?.on('data', (d: Buffer) => { stderr += d.toString() })
// ... timeout handler and close listener

Missing: No .on('error') handler on the ChildProcess.

If spawn fails (binary not found, permission denied), Node.js emits an 'error' event. Without a handler, this crashes the API handler instead of returning graceful error response.

Scenario: All candidate paths fail access() check → fallback path doesn't exist → spawn() errors → unhandled exception crashes the route.

Fix: Add error handler before timeout:

child.on('error', (err) => {
  clearTimeout(timer)
  resolve({ ok: false, stdout, stderr: `spawn error: ${err.message}` })
})

[Anonymous]

Originally posted by: adityaharishch

Inline: src/intelligence/oauth/flow.rs:145 + jira.rs:88 — HIGH

Both post_token() and discover_cloud() create HTTP clients without timeouts:

// flow.rs:145
async fn post_token(...) -> Result<TokenResponse> {
    let client = reqwest::Client::new();
    // ...
}

// jira.rs:88  
async fn discover_cloud(...) -> Result<(String, String)> {
    let client = reqwest::Client::new();
    // ...
}

Issue: No timeout on HTTP requests. If Atlassian's servers are slow or unresponsive, ensure_fresh() (called during token refresh) can block the daemon's main poll loop indefinitely.

Context: src/health/jira.rs correctly uses Client::builder().timeout(Duration::from_secs(6)).

Fix: Reuse the timeout pattern in both functions:

let client = reqwest::Client::builder()
    .timeout(Duration::from_secs(6))
    .build()?;

[Anonymous]

Originally posted by: adityaharishch

Inline: src/intelligence/oauth/jira.rs:151 — MEDIUM

pub async fn ensure_fresh() -> Result<OAuthTokens> {
    let mut t = store::load("jira")?;
    if !t.is_expired(now_unix(), 120) {
        return Ok(t);
    }
    // No lock here ↓
    let resp = flow::refresh(&t.client_id, &spec(), &t.refresh_token).await?;
    t.access_token = resp.access_token;
    if !resp.refresh_token.is_empty() {
        t.refresh_token = resp.refresh_token;  // Atlassian rotates this
    }
    store::save(&t).context(...)?;
    Ok(t)
}

Race condition: No locking on the load-refresh-save sequence.

Scenario: PM sync and health check both call ensure_fresh() concurrently → both load same refresh_token → both POST to Atlassian → both get 401 (token rotated by first request) → second caller persists invalid token → subsequent auth fails.

Impact: Self-heals on next poll, but causes transient auth-failed warnings during normal operation.

Fix: Serialize token refreshes using tokio::sync::Mutex or similar.

[Anonymous]

Originally posted by: adityaharishch

Inline: src/intelligence/oauth/store.rs:81 — LOW

let json = serde_json::to_string_pretty(tokens)?;
std::fs::write(&tmp_path, json)?;  // ← created at default umask (0644)
#[cfg(unix)]
{
    use std::os::unix::fs::PermissionsExt;
    std::fs::set_permissions(&tmp_path, std::fs::Permissions::from_mode(0o600))?;  // ← chmod AFTER
}
std::fs::rename(&tmp_path, &final_path)?;

Issue: File is world-readable for the microseconds between write() and set_permissions().

Race window: Other users on shared systems can read the Jira OAuth tokens during this brief window.

Fix: Either:

1. Set permissions before writing using open(O_CREAT | O_EXCL) with mode 0o600
2. Create parent directory with mkdir(0o700) first, then write

This is low severity (narrow window, home directory context), but worth fixing while touching this code.

[Anonymous]

Originally posted by: adityaharishch

Review Summary

Reviewed: PR [#213] (OAuth PKCE for Jira) — 35 files, ~2500 LOC

Findings: 5 issues found (2 critical, 1 high, 1 medium, 1 low)

• ✅ PKCE implementation is correct (entropy, S256, state validation)
• ✅ OAuth flow structure is well-designed
• ✅ Token store rotation awareness is correct
• ✅ Config resolution fallback logic maintains backward compatibility

Critical path: The two critical issues both affect the recommended zero-config OAuth path:

1. Silent feature disable (jira_update_enabled) — tasks sync but worklog posting disabled
2. API crash on binary not found (missing error handler) — route crashes instead of error response

Recommendation: Fix the two critical issues before merging. The high and medium severity issues are important but not merge-blockers (timeouts can be added in follow-up, concurrency guard is edge case).

[Anonymous]

Originally posted by: Akarsh-Hegde

✅ All review issues resolved

Addressed all 5 feedback items from the code review:

Severity	Issue	Fix	Commit
CRITICAL	Zero-config OAuth users had jira_update_enabled silently false	Added store::exists("jira") check to the jira_configured gate in config.rs:300	6b8be49
CRITICAL	Missing error handler on spawn() in tasks/sync API crashes on missing binary	Added .on('error') listener before timeout handler in route.ts:36	6b8be49
HIGH	OAuth HTTP clients (post_token(), discover_cloud()) had no timeout, could block daemon indefinitely	Added 6s timeout to both reqwest clients in flow.rs:145 and jira.rs:92	6b8be49
MEDIUM	Concurrent ensure_fresh() calls cause race condition → lost refresh tokens → 401 errors	Added tokio::sync::Mutex to serialize all token refreshes in jira.rs:155	6b8be49
LOW	Token file created world-readable before chmod (TOCTOU race)	Use OpenOptions::mode(0o600) to create file with correct perms upfront in store.rs:74	6b8be49

Verification

• ✅ All cargo tests pass (229 passed)
• ✅ All cargo clippy checks pass (no warnings)
• ✅ Pre-push suite green (fmt + clippy + test + UI build + security audit)

The fixes are pushed and ready for review.

[Anonymous]

Originally posted by: adityaharishch

✅ Verification Complete — All Issues Fixed

Reviewed the fixes in commit 6b8be49. All 5 findings are correctly addressed:

✅ CRITICAL [#1]: config.rs:300 — Zero-config OAuth disabled

Fix: Added || crate::intelligence::oauth::store::exists("jira") to the jira_configured gate

• Now correctly detects OAuth token store for zero-config users
• jira_update_enabled will be true for both legacy and OAuth paths ✓

✅ CRITICAL [#2]: tasks/sync/route.ts:36 — Missing error handler

Fix: Added child.on('error', (err) => { ... }) before timeout handler

• Spawn errors are now caught and return graceful error response ✓
• Unhandled exception crash is prevented ✓

✅ HIGH: flow.rs:145 + jira.rs — Missing HTTP timeout

Fix: Added Client::builder().timeout(Duration::from_secs(6)).build()? to both

• post_token() and discover_cloud() now have 6s timeout
• Prevents daemon blocking on slow/unresponsive OAuth provider ✓

✅ MEDIUM: jira.rs:155 — Concurrency race in ensure_fresh()

Fix: Added let _guard = refresh_lock().lock().await; to serialize refresh calls

• Concurrent token refreshes are now safely serialized
• Avoids double-refresh and token rotation loss ✓

✅ LOW: store.rs:74 — Temp file world-readable race

Fix: Changed from write-then-chmod to OpenOptions with mode(0o600) upfront

• File now created with correct permissions from the start
• Eliminates TOCTOU race window ✓

Verification:

• ✅ Code review: fixes match findings exactly
• ✅ Test results: 229 cargo tests pass
• ✅ Lint: cargo clippy passes (no warnings)
• ✅ Pre-push suite: green (all checks pass)

Ready to merge — all critical and high-priority issues resolved with comprehensive test coverage.

[Anonymous]

Ticket changed by: adityaharishch

• status: open --> closed

[Anonymous]

Originally posted by: adityaharishch

🎉 This PR is included in version 1.35.0 🎉

The release is available on:

• GitHub release
• v1.35.0
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀