Files
cve-dashboard/.kiro/specs/jira-api-compliance/design.md

272 lines
19 KiB
Markdown
Raw Normal View History

# Jira API Compliance Bugfix Design
## Overview
The Jira REST API integration in the STEAM Security Dashboard has three compliance violations blocking production approval. The `searchIssues()` function uses `POST /rest/api/2/search` instead of the required `GET` with query parameters. The `getIssue()` function performs single-issue `GET /rest/api/2/issue/{key}` calls, which are forbidden — all issue fetching must go through JQL search. JQL queries in `searchIssuesByKeys()` do not include `project = <KEY>` scoping, which is required for all search operations.
The fix converts `searchIssues()` from POST to GET with URL-encoded query parameters, refactors `getIssue()` to delegate to `searchIssues()` with a JQL query, and adds project scoping to `searchIssuesByKeys()`. The UAT test script and API documentation are updated to reflect the compliant patterns. All other functions (`createIssue`, `updateIssue`, `addComment`, `transitionIssue`, `getTransitions`, `testConnection`) and the rate limiting / inter-request delay infrastructure remain unchanged.
## Glossary
- **Bug_Condition (C)**: The condition that triggers the compliance violation — when `searchIssues()` sends a POST, when `getIssue()` sends a single-issue GET, or when JQL queries lack project scoping
- **Property (P)**: The desired behavior — `searchIssues()` uses GET with query parameters, `getIssue()` delegates to JQL search, and all JQL includes `project = <KEY>`
- **Preservation**: Existing behavior of all other Jira API functions, rate limiting, inter-request delays, blocked endpoint guards, and the `{ ok, data }` response shape that must remain unchanged
- **`searchIssues()`**: The function in `backend/helpers/jiraApi.js` that executes JQL queries against the Jira search endpoint
- **`getIssue()`**: The function in `backend/helpers/jiraApi.js` that fetches a single issue by key
- **`searchIssuesByKeys()`**: The function in `backend/helpers/jiraApi.js` that bulk-fetches issues by an array of keys using JQL
- **`JIRA_PROJECT_KEY`**: The environment variable containing the Jira project key used for project scoping in JQL queries
- **Charter compliance**: The set of Jira REST API usage rules posted by Charter that the integration must follow for production approval
## Bug Details
### Bug Condition
The bug manifests in three distinct ways: (1) `searchIssues()` sends a `POST /rest/api/2/search` with a JSON body instead of a `GET` with query parameters, (2) `getIssue()` sends a `GET /rest/api/2/issue/{key}?fields=...` which is a forbidden single-issue GET pattern, and (3) `searchIssuesByKeys()` builds JQL without a `project = <KEY>` clause.
**Formal Specification:**
```
FUNCTION isBugCondition(input)
INPUT: input of type { functionName: string, args: any[] }
OUTPUT: boolean
IF input.functionName == 'searchIssues' THEN
RETURN httpMethodUsed == 'POST'
AND requestPath == '/rest/api/2/search'
AND requestHasJsonBody == true
END IF
IF input.functionName == 'getIssue' THEN
RETURN requestPath MATCHES '/rest/api/2/issue/{key}'
AND httpMethodUsed == 'GET'
AND NOT requestPath CONTAINS '/rest/api/2/search'
END IF
IF input.functionName == 'searchIssuesByKeys' THEN
RETURN jqlQuery NOT CONTAINS 'project ='
END IF
RETURN false
END FUNCTION
```
### Examples
- **searchIssues() — current**: `searchIssues('project = VULN', { maxResults: 10 })` sends `POST /rest/api/2/search` with body `{ jql: "project = VULN", startAt: 0, maxResults: 10, fields: [...] }`. **Expected**: sends `GET /rest/api/2/search?jql=project%20%3D%20VULN&fields=summary%2Cstatus%2C...&maxResults=10&startAt=0`
- **getIssue() — current**: `getIssue('VULN-123')` sends `GET /rest/api/2/issue/VULN-123?fields=summary,status,...`. **Expected**: sends `GET /rest/api/2/search?jql=key%3D%22VULN-123%22%20AND%20project%3DVULN&fields=summary%2Cstatus%2C...&maxResults=1`
- **searchIssuesByKeys() — current**: `searchIssuesByKeys(['VULN-1', 'VULN-2'])` builds JQL `key in ("VULN-1", "VULN-2") AND updated >= -24h` without project scoping. **Expected**: JQL is `key in ("VULN-1", "VULN-2") AND updated >= -24h AND project = VULN`
- **getIssue() response shape — current**: returns `{ ok: true, data: { key, id, self, fields: {...} } }`. **Expected after fix**: still returns `{ ok: true, data: { key, id, self, fields: {...} } }` by extracting the single issue from search results
## Expected Behavior
### Preservation Requirements
**Unchanged Behaviors:**
- `createIssue()` must continue to send `POST /rest/api/2/issue` with issue fields in the JSON body
- `updateIssue()` must continue to send `PUT /rest/api/2/issue/{key}` to update a single issue
- `addComment()` must continue to send `POST /rest/api/2/issue/{key}/comment`
- `transitionIssue()` must continue to send `POST /rest/api/2/issue/{key}/transitions`
- `getTransitions()` must continue to send `GET /rest/api/2/issue/{key}/transitions`
- `testConnection()` must continue to send `GET /rest/api/2/myself`
- Rate limiter must continue to enforce 1,440 requests/day and 60 requests/minute burst limits
- Inter-request delays must continue to enforce 1s between GETs and 2s between writes
- Blocked endpoint guard must continue to reject `/rest/api/2/field` and `/rest/api/2/issue/bulk`
- `searchIssues()` must continue to return `{ ok, data: { total, issues } }` response shape
- `getIssue()` must continue to return `{ ok, data: <single-issue> }` response shape
**Scope:**
All functions that do NOT involve `searchIssues()`, `getIssue()`, or `searchIssuesByKeys()` should be completely unaffected by this fix. This includes:
- All write operations (`createIssue`, `updateIssue`, `addComment`, `transitionIssue`)
- Read operations that do not use the search endpoint (`getTransitions`, `testConnection`)
- Rate limiting and inter-request delay infrastructure
- Blocked endpoint guards
- Module exports and configuration constants
## Hypothesized Root Cause
Based on the bug description and code review, the root causes are:
1. **searchIssues() uses POST instead of GET**: The function was implemented using `jiraPost('/rest/api/2/search', body)` which sends JQL, fields, startAt, and maxResults as a JSON POST body. The Jira API supports both POST and GET for search, but the Charter reviewer requires GET with query parameters. The fix is to switch from `jiraPost` to `jiraGet` with URL-encoded query parameters.
2. **getIssue() uses single-issue GET endpoint**: The function was implemented using `jiraGet('/rest/api/2/issue/{key}?fields=...')` which is the standard Jira single-issue endpoint. The Charter reviewer forbids single-issue GET loops and requires all issue fetching to go through JQL search. The fix is to refactor `getIssue()` to call `searchIssues()` with `key = "{key}" AND project = <KEY>` and `maxResults: 1`, then extract the single issue from the results array.
3. **searchIssuesByKeys() missing project scoping**: The function builds JQL as `key in (...) AND updated >= -24h` but does not include `project = <KEY>`. The Charter compliance rules require all JQL queries to include project scoping. The fix is to append `AND project = ${JIRA_PROJECT_KEY}` to the JQL clause.
4. **UAT test script reflects non-compliant patterns**: Test case 3 ("Get Single Issue") exercises the old `getIssue()` pattern, test case 8 ("JQL Search") exercises the old POST-based `searchIssues()`, and test case 9 ("Bulk Key Search") does not verify project scoping. These need updating to reflect the compliant patterns.
5. **API documentation describes non-compliant endpoints**: The `docs/jira-api-use-cases.md` file lists `POST /rest/api/2/search` for JQL Search and `GET /rest/api/2/issue/{issueKey}?fields=...` for single-issue fetch. Both need updating to describe the compliant patterns.
## Correctness Properties
Property 1: Bug Condition — searchIssues Uses GET With Query Parameters
_For any_ JQL query string, fields array, startAt value, and maxResults value passed to `searchIssues()`, the function SHALL issue a `GET` request to `/rest/api/2/search` with URL-encoded query parameters `?jql=<encoded>&fields=<comma-separated>&maxResults=<n>&startAt=<n>` and SHALL NOT send a POST request or include a JSON body.
**Validates: Requirements 2.1**
Property 2: Bug Condition — getIssue Uses JQL Search Instead of Single-Issue GET
_For any_ issue key passed to `getIssue()`, the function SHALL delegate to `searchIssues()` with JQL `key = "{key}" AND project = <JIRA_PROJECT_KEY>` and `maxResults: 1`, and SHALL NOT send a request to `/rest/api/2/issue/{key}`.
**Validates: Requirements 2.3**
Property 3: Bug Condition — searchIssuesByKeys Includes Project Scoping
_For any_ non-empty array of issue keys passed to `searchIssuesByKeys()`, the JQL query SHALL include a `project = <JIRA_PROJECT_KEY>` clause alongside the `key in (...)` clause.
**Validates: Requirements 2.2**
Property 4: Preservation — Unchanged Functions Retain Original Behavior
_For any_ call to `createIssue()`, `updateIssue()`, `addComment()`, `transitionIssue()`, `getTransitions()`, or `testConnection()`, the fixed code SHALL produce exactly the same HTTP method, URL path, and request body as the original code, preserving all existing write and read operations that are not part of the search/fetch flow.
**Validates: Requirements 3.1, 3.2, 3.3, 3.4, 3.5, 3.6**
Property 5: Preservation — Response Shape Compatibility
_For any_ successful call to `searchIssues()`, the function SHALL continue to return `{ ok: true, data: { total, issues } }`. _For any_ successful call to `getIssue()`, the function SHALL continue to return `{ ok: true, data: <single-issue-object> }` by extracting the first element from the search results array.
**Validates: Requirements 3.10**
Property 6: Preservation — Rate Limiting and Delays Unchanged
_For any_ sequence of API calls, the rate limiter SHALL continue to enforce the 1,440 requests/day daily limit and 60 requests/minute burst limit, and inter-request delays SHALL continue to enforce 1 second between GET requests and 2 seconds between write requests.
**Validates: Requirements 3.7, 3.8, 3.9**
## Fix Implementation
### Changes Required
Assuming our root cause analysis is correct:
**File**: `backend/helpers/jiraApi.js`
**Function**: `searchIssues()`
**Specific Changes**:
1. **Switch from POST to GET**: Replace `jiraPost('/rest/api/2/search', body)` with `jiraGet('/rest/api/2/search?jql=...&fields=...&maxResults=...&startAt=...')`. The JQL string, comma-separated fields, maxResults, and startAt must all be URL-encoded using `encodeURIComponent()`.
2. **Remove JSON body construction**: The `body` object `{ jql, startAt, maxResults, fields }` is no longer needed. All parameters move to query string.
3. **Preserve response parsing**: The `res.status === 200` check and `JSON.parse(res.body)` remain unchanged since the Jira search endpoint returns the same JSON shape for both GET and POST.
**Function**: `searchIssuesByKeys()`
**Specific Changes**:
4. **Add project scoping to JQL**: Change the JQL from `key in (${keyList}) AND updated >= -24h` to `key in (${keyList}) AND updated >= -24h AND project = ${JIRA_PROJECT_KEY}`. The `JIRA_PROJECT_KEY` constant is already available in module scope.
**Function**: `getIssue()`
**Specific Changes**:
5. **Refactor to use searchIssues()**: Replace the direct `jiraGet('/rest/api/2/issue/...')` call with a call to `searchIssues()` using JQL `key = "{issueKey}" AND project = ${JIRA_PROJECT_KEY}` and `maxResults: 1`.
6. **Extract single issue from results**: When the search succeeds, extract `data.issues[0]` from the search results to return as `{ ok: true, data: <issue> }`. If no issues are found (empty results), return `{ ok: false, status: 404, body: 'Issue not found' }`.
7. **Preserve return shape**: The caller expects `{ ok: true, data: { key, id, self, fields: {...} } }` — the individual issue object from the search results array has this same shape.
---
**File**: `backend/scripts/jira-uat-test.js`
**Specific Changes**:
8. **Update test case 3 name**: Change from `'3. Get Single Issue (GET /issue/{key})'` to reflect the JQL-based pattern, e.g., `'3. Get Single Issue (JQL search)'`.
9. **Update test case 8 name**: Change from `'8. JQL Search (POST /search)'` to `'8. JQL Search (GET /search)'`.
10. **Update test case 9 assertions**: Add verification that the JQL used by `searchIssuesByKeys()` includes project scoping. The test already calls `searchIssuesByKeys()` — the underlying function change handles compliance.
11. **Add full-load test**: Add a test case that simulates a 24-hour sync cycle by calling `searchIssues()` with a project-scoped JQL and verifying the response shape.
---
**File**: `docs/jira-api-use-cases.md`
**Specific Changes**:
12. **Update JQL Search use case (8)**: Change endpoint from `POST /rest/api/2/search` to `GET /rest/api/2/search?jql=...&fields=...&maxResults=...&startAt=...`. Update the JQL pattern to include project scoping.
13. **Update Get Single Issue use case (3)**: Change from `GET /rest/api/2/issue/{issueKey}?fields=...` to describe the JQL-based pattern using `GET /rest/api/2/search?jql=key="ISSUE-KEY" AND project=<KEY>&fields=...&maxResults=1`.
14. **Update Issue Lookup use case (9)**: Same change as use case 3 — describe JQL-based lookup instead of single-issue GET.
15. **Update compliance summary table**: Change "Bulk reads via JQL" row from `POST /rest/api/2/search` to `GET /rest/api/2/search`. Add a row for single-issue fetch via JQL search.
## Testing Strategy
### Validation Approach
The testing strategy follows a two-phase approach: first, surface counterexamples that demonstrate the compliance violations on unfixed code, then verify the fix produces compliant behavior and preserves all existing functionality.
### Exploratory Bug Condition Checking
**Goal**: Surface counterexamples that demonstrate the compliance violations BEFORE implementing the fix. Confirm or refute the root cause analysis. If we refute, we will need to re-hypothesize.
**Test Plan**: Write unit tests that mock `jiraRequest` and capture the HTTP method, URL path, and body arguments. Run these tests on the UNFIXED code to observe the non-compliant patterns.
**Test Cases**:
1. **searchIssues POST detection**: Call `searchIssues()` and assert the HTTP method is `GET` — will fail on unfixed code because it uses `POST` (will fail on unfixed code)
2. **getIssue single-issue GET detection**: Call `getIssue('VULN-123')` and assert the URL path contains `/rest/api/2/search` — will fail on unfixed code because it uses `/rest/api/2/issue/VULN-123` (will fail on unfixed code)
3. **searchIssuesByKeys project scoping detection**: Call `searchIssuesByKeys(['VULN-1'])` and assert the JQL contains `project =` — will fail on unfixed code because project scoping is missing (will fail on unfixed code)
4. **searchIssues body detection**: Call `searchIssues()` and assert no JSON body is sent — will fail on unfixed code because it sends `{ jql, startAt, maxResults, fields }` (will fail on unfixed code)
**Expected Counterexamples**:
- `searchIssues()` sends `POST` with a JSON body instead of `GET` with query parameters
- `getIssue()` sends `GET /rest/api/2/issue/{key}` instead of `GET /rest/api/2/search?jql=...`
- `searchIssuesByKeys()` builds JQL without `project = <KEY>`
### Fix Checking
**Goal**: Verify that for all inputs where the bug condition holds, the fixed functions produce the expected compliant behavior.
**Pseudocode:**
```
FOR ALL input WHERE isBugCondition(input) DO
result := fixedFunction(input)
ASSERT expectedBehavior(result)
END FOR
```
Specifically:
- For any JQL string passed to `searchIssues()`, the request must be a GET with URL-encoded query parameters
- For any issue key passed to `getIssue()`, the request must go through `searchIssues()` with JQL `key = "{key}" AND project = <KEY>`
- For any key array passed to `searchIssuesByKeys()`, the JQL must include `project = <KEY>`
### Preservation Checking
**Goal**: Verify that for all inputs where the bug condition does NOT hold, the fixed code produces the same result as the original code.
**Pseudocode:**
```
FOR ALL input WHERE NOT isBugCondition(input) DO
ASSERT originalFunction(input) = fixedFunction(input)
END FOR
```
**Testing Approach**: Property-based testing is recommended for preservation checking because:
- It generates many test cases automatically across the input domain
- It catches edge cases that manual unit tests might miss
- It provides strong guarantees that behavior is unchanged for all non-buggy inputs
**Test Plan**: Observe behavior on UNFIXED code first for all unchanged functions, then write property-based tests capturing that behavior.
**Test Cases**:
1. **createIssue preservation**: Observe that `createIssue()` sends `POST /rest/api/2/issue` on unfixed code, then verify this continues after fix
2. **updateIssue preservation**: Observe that `updateIssue()` sends `PUT /rest/api/2/issue/{key}` on unfixed code, then verify this continues after fix
3. **addComment preservation**: Observe that `addComment()` sends `POST /rest/api/2/issue/{key}/comment` on unfixed code, then verify this continues after fix
4. **Response shape preservation**: Observe that `searchIssues()` returns `{ ok, data: { total, issues } }` on unfixed code, then verify the same shape after fix
5. **getIssue response shape preservation**: Observe that `getIssue()` returns `{ ok, data: <issue> }` on unfixed code, then verify the same shape after fix (extracted from search results)
6. **Rate limiter preservation**: Observe that rate limits are enforced on unfixed code, then verify they continue after fix
### Unit Tests
- Test `searchIssues()` sends GET with correctly URL-encoded query parameters for various JQL strings
- Test `searchIssues()` handles special characters in JQL (quotes, spaces, operators) via proper encoding
- Test `getIssue()` delegates to `searchIssues()` with correct JQL and `maxResults: 1`
- Test `getIssue()` extracts single issue from search results and returns `{ ok, data: <issue> }`
- Test `getIssue()` returns `{ ok: false }` when search returns empty results
- Test `searchIssuesByKeys()` includes `project = <KEY>` in JQL
- Test `searchIssuesByKeys()` with empty array returns `{ ok: true, data: { total: 0, issues: [] } }`
### Property-Based Tests
- Generate random JQL strings and verify `searchIssues()` always uses GET method with query parameters and never sends a POST body
- Generate random issue keys and verify `getIssue()` always routes through `/rest/api/2/search` with `maxResults=1` and project scoping
- Generate random arrays of issue keys and verify `searchIssuesByKeys()` always includes `project = <KEY>` in the JQL
- Generate random inputs for unchanged functions (`createIssue`, `updateIssue`, `addComment`) and verify they produce identical HTTP method, path, and body as the original implementation
### Integration Tests
- Run the UAT test script against a mock or UAT Jira instance and verify all test cases pass with compliant patterns
- Test a full 24-hour sync cycle simulation: `searchIssues()` with project-scoped JQL, verify response shape, verify rate limit accounting
- Test `getIssue()` end-to-end: call with a known key, verify the response contains the expected issue data extracted from search results
- Test `searchIssuesByKeys()` end-to-end: call with a mix of valid and invalid keys, verify project-scoped JQL and partial results handling