umbragit/UmbraAudio
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make approval visible, and fix a real key-truncation bug in the auth gate

Browse Source 
Follow-up to the badge fix: approving a speaker removed it from the
needs-check panel with no confirmation, which reads exactly like the
speaker being deleted. Reported as "the speaker advisor isn't shown now".

- A person-approved speaker now renders "✓ reviewed" with a tooltip saying
  so. It previously rendered "✓ clear" whose tooltip claimed
  "auto-accepted" - a false claim about a decision a machine never made.
- Approving confirms itself with a toast, drops the entry from the
  worklist, and updates the header counts, all without a reload.

Separately, a pre-existing flake in tests/test_auth.py turned out to be a
real defect rather than test noise:

  _session_secret() read the key file with .strip(), but bytes.strip()
  removes ASCII whitespace from both ends. A secret beginning or ending
  with one of those six bytes (4.35% of generated keys, measured) came back
  one byte shorter on every read after the write - a different HMAC key,
  silently, so a token signed before the flip could never be verified again.
  Now read verbatim, with a length floor to reject a corrupt file.

That test also lacked the isolated_store fixture, so it read and rewrote the
real .umbra/session_secret - running the suite mutated production state.
This install's key is 32 bytes with no whitespace at either end, so no
existing session is invalidated by the fix.

Also verified: Advisor remains at display position 1 of 91 with its
utterance (utt 3668, ch15) intact - only the warning badge changed.

auth.py test flake 4/25 runs -> 0/30. Suite: 684 passed, 8 pre-existing.
This commit is contained in:
B.D. Murphy
2026-10-10 19:18:04 -07:00
parent a4c88f8bde
commit 2fda85f9d3
5 changed files with 100 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
tests/test_auth.py
View File

@@ -7,6 +7,7 @@ store, the password rule, and the tamper-resistance of the session token.
from __future__ import annotations
import json
import secrets
import time
from pathlib import Path
@@ -144,8 +145,47 @@ def test_session_payload_carries_no_password_material() -> None:
assert set(payload) == {"u", "exp"}
def test_session_secret_is_stable_across_calls() -> None:
def test_session_secret_is_stable_across_calls(isolated_store: Path) -> None:
# The fixture is load-bearing: without it this test read and rewrote the
# REAL .umbra/session_secret, so running the suite mutated production state.
a = auth._session_secret()
b = auth._session_secret()
assert a == b
assert len(a) == 32
assert len(a) == 32
def test_secret_with_whitespace_bytes_is_returned_verbatim(isolated_store: Path) -> None:
"""Regression: ``.strip()`` silently shortened ~4% of generated secrets.
``bytes.strip()`` removes ASCII whitespace from both ends, so a secret
beginning or ending with one of those six bytes was returned one byte
shorter on the next read than the value just written - a different key.
"""
for lead in (b"", b" ", b"\t", b"\n", b"\r", b"\x0b", b"\x0c"):
for tail in (b"", b" ", b"\t", b"\n", b"\r", b"\x0b", b"\x0c"):
raw = lead + secrets.token_bytes(32) + tail
path = auth._secret_file()
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(raw)
assert auth._session_secret() == raw, f"mutated for {lead!r}...{tail!r}"
def test_truncated_secret_is_regenerated(isolated_store: Path) -> None:
path = auth._secret_file()
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(b"tiny")
got = auth._session_secret()
assert len(got) == auth.SECRET_BYTES
assert got != b"tiny"
def test_token_survives_a_round_trip_with_whitespace_secret(
isolated_store: Path,
) -> None:
"""The user-visible symptom: a session that can never be verified."""
raw = b" " + secrets.token_bytes(32) + b"\n"
path = auth._secret_file()
path.parent.mkdir(parents=True, exist_ok=True)
path.write_bytes(raw)
token = auth.issue_session(auth.USERNAME)
assert auth.verify_session(token) is not None

15
umbraaudio/auth.py
View File

@@ -51,6 +51,9 @@ MIN_PASSWORD_LENGTH = 7
AUTH_FILE_NAME = "auth.json"
SECRET_FILE_NAME = "session_secret"
SECRET_BYTES = 32
# A shorter file is treated as corrupt and regenerated rather than used as a key.
MIN_SECRET_BYTES = 16
def _store_dir() -> Path:
@@ -237,12 +240,18 @@ def _session_secret() -> bytes:
path = _secret_file()
if path.exists():
try:
data = path.read_bytes().strip()
if data:
# Read verbatim. ``bytes.strip()`` removes ASCII whitespace from both
# ends, so a secret that happened to start or end with one of those
# six bytes came back one byte shorter on every subsequent call than
# the value that had just been written - a different key, silently,
# for ~4% of installs. Tokens signed before that flip could never be
# verified again.
data = path.read_bytes()
if len(data) >= MIN_SECRET_BYTES:
return data
except OSError:
pass
secret = secrets.token_bytes(32)
secret = secrets.token_bytes(SECRET_BYTES)
path.write_bytes(secret)
with contextlib.suppress(OSError):
path.chmod(0o600)

8
umbraaudio/web/templates/_speaker_row.html
View File

@@ -6,8 +6,16 @@
{% if _desig %}
{% if _desig.status == 'clear' %}
<span class="qa-badge-wrap" style="margin-left:0.3rem">
{% if _desig.reviewed %}
{# A person approved this speaker. Do not label it auto-accepted -
that claim is false here, and it is what made an approved row
look like it had simply been dropped from the worklist. #}
<span class="badge badge-green" style="font-size:0.65rem"
title="You approved this speaker — no longer in the needs-check list.{{ '\n' }}Still flagged for:{% if _desig.reasons %}&#10;{% for r in _desig.reasons %}{{ loop.index }}. {{ r }}{% if not loop.last %}&#10;{% endif %}{% endfor %}{% else %} none{% endif %}">✓ reviewed</span>
{% else %}
<span class="badge badge-green" style="font-size:0.65rem"
title="Identified with {{ '%.0f'|format(_desig.confidence * 100) }}% confidence — auto-accepted">✓ clear</span>
{% endif %}
</span>
{% else %}
<span class="qa-badge-wrap" style="margin-left:0.3rem">

8
umbraaudio/web/templates/_speakers_list.html
View File

@@ -6,8 +6,8 @@
{% set _clear = designations.values()|selectattr('status', 'equalto', 'clear')|list|length %}
{% set _needs = designations.values()|selectattr('status', 'equalto', 'needs_review')|list|length %}
{% if designations %}
<span class="badge badge-green" style="font-size:0.7rem; vertical-align:middle" title="Auto-accepted: identified with high confidence">{{ _clear }} clear</span>
<span class="badge badge-red" style="font-size:0.7rem; vertical-align:middle" title="A human should confirm these">{{ _needs }} need checking</span>
<span class="badge badge-green" style="font-size:0.7rem; vertical-align:middle" data-vq-clear-count title="Auto-accepted: identified with high confidence">{{ _clear }} clear</span>
<span class="badge badge-red" style="font-size:0.7rem; vertical-align:middle" data-vq-needs-count title="A human should confirm these">{{ _needs }} need checking</span>
{% endif %}
</h3>
<div style="display:flex; gap:0.5rem; align-items:center">
@@ -25,7 +25,7 @@
{% if _needs %}
<div style="padding:0.85rem 1rem; margin-bottom:1rem; background:#1a1a2e; border:1px solid #7f1d1d; border-left:4px solid #ef4444; border-radius:6px">
<strong style="color:#ef4444">⚠ {{ _needs }} speaker{{ 's' if _needs != 1 }} need{{ '' if _needs != 1 }} checking</strong>
<strong style="color:#ef4444" data-vq-needs-heading>⚠ {{ _needs }} speaker{{ 's' if _needs != 1 }} need{{ '' if _needs != 1 }} checking</strong>
<span style="color:#94a3b8; font-size:0.85rem"> — these were not auto-accepted as distinct speakers. For each one:</span>
<ol style="color:#cbd5e1; font-size:0.84rem; margin:0.5rem 0 0 1.1rem; line-height:1.6">
<li>Hover the <span class="badge badge-red" style="font-size:0.65rem">⚠ needs check</span> badge on the row to see
@@ -41,7 +41,7 @@
<summary style="cursor:pointer; color:#93c5fd; font-size:0.82rem">Show the list</summary>
<div style="margin-top:0.5rem; max-height:260px; overflow-y:auto">
{% for _n in designations.values()|sort(attribute='confidence')|selectattr('status', 'equalto', 'needs_review')|list %}
<div style="padding:0.3rem 0; border-bottom:1px solid #1e293b; font-size:0.82rem">
<div data-review-item="{{ _n.name }}" style="padding:0.3rem 0; border-bottom:1px solid #1e293b; font-size:0.82rem">
<strong style="color:#e0e0e0">{{ _n.name }}</strong>
<span style="color:#64748b">({{ '%.0f'|format(_n.confidence * 100) }}%)</span>
<div style="color:#94a3b8">

34
umbraaudio/web/templates/speakers.html
View File

@@ -185,6 +185,7 @@ function approveSpeakerVoice(name, idx) {
target: '#speaker-row-' + rowIdx,
swap: 'outerHTML'
});
markApproved(name);
} else {
alert('Failed: ' + (data.error || 'unknown'));
}
@@ -317,3 +318,36 @@ function refreshVoiceQueueTags() {
document.addEventListener('DOMContentLoaded', refreshVoiceQueueTags);
</script>
{% endblock %}
// Approving used to be silent: the speaker simply stopped appearing in the
// needs-check panel, which reads exactly like the speaker being deleted. Say
// what happened, keep the counts honest, and drop the entry from the worklist.
function markApproved(name) {
var toast = document.createElement('div');
toast.textContent = '\u2713 ' + name + ' approved \u2014 still in the speaker list, just no longer needing review.';
toast.style.cssText = 'position:fixed; bottom:1.2rem; right:1.2rem; z-index:1100; ' +
'background:#065f46; color:#ecfdf5; padding:0.6rem 0.9rem; border-radius:6px; ' +
'font-size:0.84rem; box-shadow:0 4px 12px rgba(0,0,0,0.4); max-width:22rem';
document.body.appendChild(toast);
setTimeout(function () { toast.remove(); }, 5000);
var entry = document.querySelector('[data-review-item="' + CSS.escape(name) + '"]');
if (entry) entry.remove();
var needs = document.querySelector('[data-vq-needs-count]');
if (needs) {
var n = Math.max(0, (parseInt(needs.textContent, 10) || 0) - 1);
needs.textContent = n + ' need checking';
}
var clear = document.querySelector('[data-vq-clear-count]');
if (clear) clear.textContent = ((parseInt(clear.textContent, 10) || 0) + 1) + ' clear';
var head = document.querySelector('[data-vq-needs-heading]');
if (head) {
var left = (parseInt(head.getAttribute('data-left'), 10));
if (isNaN(left)) left = (parseInt(head.textContent, 10) || 1);
head.setAttribute('data-left', left - 1);
head.textContent = '\u26a0 ' + Math.max(0, left - 1) + ' speaker' +
(left - 1 === 1 ? '' : 's') + ' need' + (left - 1 === 1 ? 's' : '') + ' checking';
}
}