Change, withdraw or resubmit your own exception, and find one by id

The person who asked for an exception can now change its reason, expiry,
ticket or kind from the finding page, and withdraw it while it still waits.
A rejected or withdrawn one shows the approver's note and an Update and
resubmit button, which sends it back for approval with the old decision
cleared.  An approved one cannot be changed on the screen; an administrator
revokes it.  Nobody else can change or withdraw a request that is not theirs,
administrators included: it answers 404.

The Exceptions page shows each exception's id (E77), who approved or rejected
it and their note, and Change or Update and resubmit and Withdraw buttons on
your own rows.  Its search takes an exception id: 77 matches the exception or
the finding, E77 or "exception 77" only the exception, #77 only the finding.

In the token API, GET /exceptions takes id, one exception number or up to 500
separated by commas, and vuln_id, the finding's GCR id.  PATCH on a rejected
or withdrawn exception now resubmits it instead of answering 409.  The API
reference, OpenAPI and Postman files, the Python and bash samples and the
changelog cover it.

Author: hackrange

app/core/api_reference.py

@@ -234,7 +234,11 @@ def endpoints() -> list:
                           choices=["requested", "approved", "rejected",
                                    "revoked", "expired"]),
                        _p("finding", "Only the exception on this finding number.",
-                          kind="integer")] + PAGING,
+                          kind="integer"),
+                       _p("id", "Only these exception numbers.  One, or up to 500 "
+                                "separated by commas, like 77,81."),
+                       _p("vuln_id", "Only exceptions on the finding with this GCR "
+                                     "id, like GCR-1A2B3C4D5E6F.")] + PAGING,
             "example": {"total": 1, "limit": 100, "offset": 0, "count": 1, "exceptions": [{
                 "id": 77, "finding": 4182, "status": "requested", "category": "",
                 "reason": "Only reachable from the admin network, fix planned in Q1.",
@@ -303,8 +307,8 @@ def endpoints() -> list:
                            "waiting one stays waiting.  An approved one goes "
                            "back to waiting and its finding returns to the "
                            "report files until an administrator approves it "
-                           "again.  Answers 409 on one that is rejected or "
-                           "revoked: ask again instead.",
+                           "again.  A rejected or withdrawn one goes back to "
+                           "waiting, the same as asking again.",
             "params": [_p("exception_id", "The exception number.", where="path",
                           kind="integer", example=77)],
             "body": [

app/web/routes_api.py

@@ -669,6 +669,120 @@ async def revoke_waiver(waiver_id: int, request: Request,
                     + "&kind=warn")
 
 
+def _own_unapproved_waiver(db, user, waiver_id, allowed):
+    """(waiver, finding) for a request this person made, in one of these states.
+
+    Somebody else's answers 404, so an id says nothing about whether it
+    exists.  Approved ones are never in the list: what was signed off stays as
+    it was until an administrator revokes it.
+    """
+    waiver = db.query(Waiver).filter(Waiver.id == waiver_id).one_or_none()
+    if waiver is None or ((waiver.requested_by or "").lower()
+                          != (user.username or "").lower()):
+        raise HTTPException(status_code=404,
+                            detail="No exception request of yours with that id.")
+    finding = rbac.visible_finding(db, user, waiver.finding_id)
+    if finding is None:
+        raise HTTPException(status_code=404,
+                            detail="No exception request of yours with that id.")
+    if waiver.status not in allowed:
+        raise HTTPException(
+            status_code=409,
+            detail=("That cannot be done to an exception that is "
+                    + waiver.status.value + "."
+                    + ("  An administrator can revoke it."
+                       if waiver.status == WaiverStatus.APPROVED else "")))
+    return waiver, finding
+
+
+@router.post("/waivers/{waiver_id}/edit")
+async def edit_own_waiver(waiver_id: int, request: Request,
+                          user: User = Depends(current_user)):
+    """Change your own request: reason, expiry, ticket, kind.
+
+    Anything not approved.  One that was rejected or withdrawn goes back to
+    waiting, the same as asking again, with the old decision cleared.
+    """
+    await check_csrf(request)
+    rbac.require(user, "waiver.request")
+
+    from urllib.parse import quote
+
+    form = await request.form()
+    reason_text = clean_text(form.get("reason", ""), "Reason", limit=2000)
+    if len(reason_text) < 20:
+        raise HTTPException(
+            status_code=400,
+            detail="Say a bit more about why this is acceptable, at least 20 characters.")
+    ticket_text = clean_text(form.get("ticket", ""), "Ticket", limit=120,
+                             required=False)
+    expires_at = _parse_expiry(str(form.get("expires", "")))
+    category_value = _waiver_category(form.get("category", ""))
+
+    with session_scope() as db:
+        waiver, finding = _own_unapproved_waiver(
+            db, user, waiver_id,
+            {WaiverStatus.REQUESTED, WaiverStatus.REJECTED, WaiverStatus.REVOKED})
+        reopened = waiver.status != WaiverStatus.REQUESTED
+        if category_value == "not_deployed" and finding.kind == FindingKind.CREDENTIAL:
+            raise HTTPException(status_code=400, detail=(
+                "A credential works wherever it was pushed, deployed or not.  "
+                "Rotate it, or ask for an accepted risk exception instead."))
+        changed = [name for name, old, new in (
+            ("reason", waiver.reason or "", reason_text),
+            ("ticket", waiver.ticket or "", ticket_text),
+            ("expires", waiver.expires_at, expires_at),
+            ("category", waiver.category or "", category_value)) if old != new]
+        waiver.reason = reason_text
+        waiver.ticket = ticket_text
+        waiver.expires_at = expires_at
+        waiver.category = category_value
+        waiver.requested_at = utcnow()
+        if reopened:
+            waiver.status = WaiverStatus.REQUESTED
+            waiver.decided_by = ""
+            waiver.decided_at = None
+            waiver.decision_note = ""
+        db.flush()
+        record(db, "waiver.edited", actor=user.username,
+               target=finding.fingerprint,
+               detail=((", ".join(changed) or "nothing") + " changed"
+                       + ("; sent for approval again" if reopened else "")),
+               source_ip=client_ip(request))
+        where = _back_to(form, finding.id, "/waivers")
+
+    return redirect(f"{where}?msg="
+                    + quote("Request changed.  It is still waiting for an administrator.")
+                    + "&kind=ok")
+
+
+@router.post("/waivers/{waiver_id}/withdraw")
+async def withdraw_own_waiver(waiver_id: int, request: Request,
+                              user: User = Depends(current_user)):
+    """Take back your own request before anybody decides on it.
+
+    Kept as revoked with a note, like the API's withdraw, so the finding can
+    be asked about again and the record of the request stays readable.
+    """
+    await check_csrf(request)
+    rbac.require(user, "waiver.request")
+
+    from urllib.parse import quote
+
+    form = await request.form()
+    with session_scope() as db:
+        waiver, finding = _own_unapproved_waiver(db, user, waiver_id,
+                                                 {WaiverStatus.REQUESTED})
+        take_back_waiver(db, waiver, finding, user.username,
+                         note="Withdrawn by the person who asked for it.")
+        db.flush()
+        record(db, "waiver.withdrawn", actor=user.username,
+               target=finding.fingerprint, source_ip=client_ip(request))
+        where = _back_to(form, finding.id, "/waivers")
+
+    return redirect(f"{where}?msg=" + quote("Request withdrawn.") + "&kind=info")
+
+
 def form_ids(form, *names) -> list:
     """Whole numbers out of a form, under any of these field names.
 

app/web/routes_api_v1.py

@@ -633,6 +633,23 @@ def list_exceptions(request: Request,
                 raise HTTPException(status_code=400,
                                     detail="finding is a finding number.")
             base = base.filter(Waiver.finding_id == int(finding_raw))
+        id_raw = (params.get("id", "") or "").strip()
+        if id_raw:
+            ids = [part.strip() for part in id_raw.split(",") if part.strip()]
+            if not ids or len(ids) > 500 or not all(i.isdigit() for i in ids):
+                raise HTTPException(
+                    status_code=400,
+                    detail="id is an exception number, or up to 500 of them "
+                           "separated by commas.")
+            base = base.filter(Waiver.id.in_({int(i) for i in ids}))
+        vuln_raw = (params.get("vuln_id", "") or "").strip().upper()
+        if vuln_raw:
+            if len(vuln_raw) > 64:
+                raise HTTPException(status_code=400,
+                                    detail="vuln_id is a GCR id, like GCR-1A2B3C4D5E6F.")
+            base = base.filter(Waiver.finding_id.in_(
+                shown.filter(Finding.vuln_id == vuln_raw)
+                .with_entities(Finding.id).scalar_subquery()))
 
         total = base.count()
         rows = (base.order_by(Waiver.requested_at.desc(), Waiver.id.desc())
@@ -777,7 +794,8 @@ async def edit_exception(exception_id: int, request: Request,
     approver cannot rewrite somebody else's argument under their name.  An
     approved exception that is changed goes back to waiting: what was signed
     off is not what it says any more, so its finding comes back into the
-    report files until an administrator agrees again.
+    report files until an administrator agrees again.  A rejected or withdrawn
+    one goes back to waiting too, the same as asking again.
     """
     from app.core.audit import record
     from app.core.models import FindingKind, WaiverStatus, utcnow
@@ -806,11 +824,11 @@ async def edit_exception(exception_id: int, request: Request,
         if not _is_owner(me, waiver):
             raise HTTPException(status_code=403,
                                 detail="Only the person who asked for it can change it.")
-        if waiver.status not in {WaiverStatus.REQUESTED, WaiverStatus.APPROVED}:
-            raise HTTPException(
-                status_code=409,
-                detail="That one is closed.  Ask again with POST "
-                       "/findings/{id}/exceptions.")
+        if waiver.status not in {WaiverStatus.REQUESTED, WaiverStatus.APPROVED,
+                                 WaiverStatus.REJECTED, WaiverStatus.REVOKED}:
+            raise HTTPException(status_code=409,
+                                detail="That one cannot be changed.")
+        reopened = waiver.status in {WaiverStatus.REJECTED, WaiverStatus.REVOKED}
 
         if "reason" in body:
             reason = clean_text(body.get("reason") or "", "Reason", limit=2000)
@@ -846,6 +864,7 @@ async def edit_exception(exception_id: int, request: Request,
         record(db, "waiver.edited", actor=me.username, target=finding.fingerprint,
                detail=(f"{', '.join(sorted(body))} changed"
                        + ("; approval undone, waiting again" if was_approved else "")
+                       + ("; sent for approval again" if reopened else "")
                        + f" (api token {caller.token.prefix})"),
                source_ip=client_ip(request))
         if was_approved:

app/web/routes_pages.py

@@ -2018,11 +2018,20 @@ def waivers(request: Request, q: str = "", status: str = "", page_no: int = 1,
                       Waiver.ticket.ilike(needle),
                       Waiver.requested_by.ilike(needle),
                       Waiver.finding_id.in_(matching)]
-            # A finding number on its own, as 4182 or #4182, is that finding.
-            number = query.lstrip("#").strip()
-            if number.isdigit():
-                either.append(Waiver.finding_id == int(number))
-            base = base.filter(sa_or(*either))
+            # A number on its own, as 4182, is that finding or the exception
+            # with that id.  #4182 is only the finding, and E77 or
+            # "exception 77" is only the exception, for when both exist.
+            only_exception = re.fullmatch(r"(?:e|ex|exc|exception)\s*#?\s*(\d+)",
+                                          query.strip(), re.IGNORECASE)
+            if only_exception:
+                base = base.filter(Waiver.id == int(only_exception.group(1)))
+            else:
+                if query.strip().isdigit():
+                    either.append(Waiver.id == int(query.strip()))
+                number = query.lstrip("#").strip()
+                if number.isdigit():
+                    either.append(Waiver.finding_id == int(number))
+                base = base.filter(sa_or(*either))
 
         total = base.count()
         pages = max(1, (total + per - 1) // per)

app/web/templates/finding_detail.html

@@ -1091,14 +1091,70 @@ <h1><span class="badge {{ f.severity.value }}"
 {% endif %}
 
 {% if waiver %}
-<div class="card">
+<div class="card" id="exception">
   <div class="card-head">Exception</div>
   <div class="card-body">
-    <p><span class="badge {{ 'pending' if waiver.status.value == 'requested' else 'ok' }}">{{ waiver.status.value }}</span></p>
+    <p><span class="badge {{ 'pending' if waiver.status.value == 'requested' else ('ok' if waiver.status.value == 'approved' else 'waived') }}">{{ waiver.status.value }}</span>
+      <span class="faint small mono">E{{ waiver.id }}</span></p>
     <p>{{ waiver.reason }}</p>
     <p class="small faint">Asked for by {{ waiver.requested_by }},
       expires {{ waiver.expires_at | dt('%Y-%m-%d') if waiver.expires_at }}
       {% if waiver.decided_by %}, decided by {{ waiver.decided_by }}{% endif %}</p>
+    {% if waiver.decision_note %}
+    <p class="small"><strong>{{ 'Why it was rejected' if waiver.status.value == 'rejected' else 'Note' }}:</strong>
+      {{ waiver.decision_note }}</p>
+    {% endif %}
+    {# The person who asked can change their own request until somebody
+       approves it, and take it back while it is still waiting.  A rejected or
+       withdrawn one that is changed goes back for approval. #}
+    {% if can_waive and waiver.status.value != 'approved'
+          and (waiver.requested_by or '') | lower == (user.username or '') | lower %}
+    <details style="margin-top:.5rem" {{ 'open' if waiver.status.value == 'rejected' }}>
+      <summary class="small">{{ 'Change this request' if waiver.status.value == 'requested'
+                                else 'Update and resubmit' }}</summary>
+      <form method="post" action="{{ base }}/waivers/{{ waiver.id }}/edit"
+            style="margin-top:.5rem;max-width:70ch">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token }}">
+        <input type="hidden" name="back" value="finding">
+        {% if f.kind.value != 'credential' %}
+        <div class="field">
+          <label for="edit-category">Kind of exception</label>
+          <select id="edit-category" name="category">
+            <option value="" {{ 'selected' if not waiver.category }}>Accepted risk</option>
+            <option value="not_deployed" {{ 'selected' if waiver.category == 'not_deployed' }}>Not in scope: this code does not get deployed</option>
+          </select>
+        </div>
+        {% endif %}
+        <div class="field">
+          <label for="edit-reason">Why is this acceptable</label>
+          <textarea id="edit-reason" name="reason" required minlength="20"
+                    maxlength="2000">{{ waiver.reason }}</textarea>
+        </div>
+        <div class="field">
+          <label for="edit-expires">Expires</label>
+          <input type="date" id="edit-expires" name="expires" required
+                 min="{{ expiry_min }}" max="{{ expiry_max }}"
+                 value="{{ waiver.expires_at | dt('%Y-%m-%d') if waiver.expires_at else default_expiry }}">
+        </div>
+        <div class="field">
+          <label for="edit-ticket">Ticket</label>
+          <input type="text" id="edit-ticket" name="ticket" maxlength="120"
+                 value="{{ waiver.ticket or '' }}">
+        </div>
+        <button class="btn sm primary" type="submit">{{ 'Save' if waiver.status.value == 'requested'
+                                               else 'Resubmit for approval' }}</button>
+      </form>
+    </details>
+    {% if waiver.status.value == 'requested' %}
+    <form method="post" action="{{ base }}/waivers/{{ waiver.id }}/withdraw"
+          style="margin-top:.5rem"
+          data-confirm="Withdraw this request? You can ask again later.">
+      <input type="hidden" name="csrf_token" value="{{ csrf_token }}">
+      <input type="hidden" name="back" value="finding">
+      <button class="btn sm danger" type="submit">Withdraw request</button>
+    </form>
+    {% endif %}
+    {% endif %}
     {% if can_revoke and waiver.status.value == 'approved' %}
     <form method="post" action="{{ base }}/waivers/{{ waiver.id }}/revoke"
           data-confirm="The finding comes straight back into both report files. Continue?">

app/web/templates/waivers.html

@@ -26,7 +26,7 @@
   <div class="field" style="flex:1;min-width:220px">
     <label for="q">Search</label>
     <input type="text" id="q" name="q" value="{{ q }}"
-           placeholder="finding number, GCR id, repository, path, reason, ticket or who asked">
+           placeholder="finding number, exception id (E77), GCR id, repository, path, reason, ticket or who asked">
   </div>
   <div class="field">
     <label for="status">Status</label>
@@ -91,7 +91,7 @@
         <th class="nowrap"><input type="checkbox" id="bulk-select-page"
             title="Everything on this page you can act on"></th>
         {% endif %}
-        <th class="nowrap" title="The finding number">#</th><th>Status</th><th>Finding</th><th>Why</th><th>Asked by</th><th></th></tr></thead>
+        <th class="nowrap" title="The finding number, and the exception id under it">#</th><th>Status</th><th>Finding</th><th>Why</th><th>Asked by</th><th></th></tr></thead>
       <tbody>
       {% for w in rows %}
         {% set f = findings.get(w.finding_id) %}
@@ -107,8 +107,9 @@
                      data-state="{{ w.status.value }}"
                      {{ 'disabled' if not can_act }}></td>
           {% endif %}
-          <td class="nowrap mono"><a href="{{ base }}/findings/{{ w.finding_id }}"
-              title="Open finding {{ w.finding_id }}">{{ w.finding_id }}</a></td>
+          <td class="stack nowrap mono"><a href="{{ base }}/findings/{{ w.finding_id }}"
+              title="Open finding {{ w.finding_id }}">{{ w.finding_id }}</a>
+            <span class="sub" title="The exception id, as the API and E{{ w.id }} in the search know it">E{{ w.id }}</span></td>
           <td class="stack nowrap">
             {% if w.status.value == 'approved' and w.expires_at and w.expires_at.replace(tzinfo=None) < now.replace(tzinfo=None) %}
               <span><span class="badge medium">expired</span></span>
@@ -143,7 +144,8 @@
           </td>
           <td class="stack nowrap">
             <span>{{ w.requested_by }}</span>
-            {% if w.decided_by %}<span class="sub">decided by {{ w.decided_by }}</span>{% endif %}
+            {% if w.decided_by %}<span class="sub">{{ w.status.value if w.status.value in ('approved', 'rejected') else 'decided' }} by {{ w.decided_by }}</span>{% endif %}
+            {% if w.decision_note %}<span class="sub" style="white-space:normal;max-width:40ch">{{ w.decision_note }}</span>{% endif %}
           </td>
           <td class="nowrap">
             {% if can_approve and w.status.value == 'requested' %}
@@ -165,6 +167,22 @@
                 <button class="btn sm danger" type="submit">Revoke</button>
               </form>
             {% endif %}
+            {# The person who asked: change it until it is approved, and
+               withdraw it while it still waits.  The form is on the finding. #}
+            {% if w.status.value != 'approved'
+                  and (w.requested_by or '') | lower == (user.username or '') | lower %}
+              <a class="btn sm{{ ' primary' if w.status.value == 'rejected' }}"
+                 href="{{ base }}/findings/{{ w.finding_id }}#exception"
+                 title="Change the reason, expiry or ticket">{{ 'Change' if w.status.value == 'requested' else 'Update and resubmit' }}</a>
+              {% if w.status.value == 'requested' %}
+              <form method="post" action="{{ base }}/waivers/{{ w.id }}/withdraw"
+                    style="display:inline"
+                    data-confirm="Withdraw this request? You can ask again later.">
+                <input type="hidden" name="csrf_token" value="{{ csrf_token }}">
+                <button class="btn sm danger" type="submit">Withdraw</button>
+              </form>
+              {% endif %}
+            {% endif %}
           </td>
         </tr>
       {% endfor %}