feat(523): forge-connection tokens are stored sealed under SECRET_KEY, like the backup S3 secret
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 56s
CI & Build / integration (push) Successful in 1m8s
CI & Build / Python tests (push) Successful in 1m54s
CI & Build / Build & push image (push) Successful in 36s

The column becomes token_sealed (migration 0122 seals existing rows with a
frozen copy of the v1 key), every reader opens it through connection_token,
a row this instance cannot open is skipped with a warning, and Settings asks
for that token again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-15 18:08:11 -04:00
co-authored by Claude Opus 5.5
parent 3008d29f89
commit 07bc51b890
10 changed files with 232 additions and 16 deletions
@@ -0,0 +1,82 @@
"""Seal forge-connection tokens at rest (milestone 523)
Revision ID: 0122
Revises: 0121
Create Date: 2026-10-15
A forge token was stored as plain text, so a copy of the database (or of a
backup tool's snapshot of it) was enough to read every user's forge
credential. Tokens are now sealed the way the backup destination's S3 secret
is (services/secrets.py): Fernet under a key derived from SECRET_KEY, which
lives in the deployment's environment rather than in the data.
The column is renamed `token` -> `token_sealed` in the same step. The name
says what the bytes are, and every reader has to change with it, so a reader
missed here fails on a missing attribute instead of quietly sending
ciphertext to the forge as a token.
THE KEY DERIVATION IS FROZEN HERE, not imported. This revision must seal with
exactly v1's key forever; importing services.secrets would let a later edit
there change what an old migration writes. The unit lane checks that what
this produces is what `secrets.unseal` opens today.
Cost: one row per (user, forge host) — a handful per instance.
Downgrade opens each token back into plain text. A row sealed under a
different SECRET_KEY cannot be opened and comes back empty; its owner enters
the token again.
"""
import base64
import hashlib
import sqlalchemy as sa
from alembic import op
revision = "0122"
down_revision = "0121"
branch_labels = None
depends_on = None
_LABEL_V1 = b"scribe.secrets.v1\x00"
def _fernet():
from cryptography.fernet import Fernet
from scribe.config import Config
digest = hashlib.sha256(_LABEL_V1 + Config.SECRET_KEY.encode()).digest()
return Fernet(base64.urlsafe_b64encode(digest))
def seal_v1(plain: str) -> str:
return _fernet().encrypt(plain.encode()).decode()
def upgrade() -> None:
op.alter_column("forge_connections", "token", new_column_name="token_sealed")
bind = op.get_bind()
rows = bind.execute(sa.text("SELECT id, token_sealed FROM forge_connections")).all()
for row_id, plain in rows:
bind.execute(
sa.text("UPDATE forge_connections SET token_sealed = :v WHERE id = :id"),
{"v": seal_v1(plain or ""), "id": row_id},
)
def downgrade() -> None:
from cryptography.fernet import InvalidToken
bind = op.get_bind()
fernet = _fernet()
rows = bind.execute(sa.text("SELECT id, token_sealed FROM forge_connections")).all()
for row_id, sealed in rows:
try:
plain = fernet.decrypt((sealed or "").encode()).decode()
except (InvalidToken, ValueError):
plain = ""
bind.execute(
sa.text("UPDATE forge_connections SET token_sealed = :v WHERE id = :id"),
{"v": plain, "id": row_id},
)
op.alter_column("forge_connections", "token_sealed", new_column_name="token")
+3 -1
View File
@@ -26,7 +26,9 @@ differently. It does **not** hold:
- **Search indexes.** They are rebuilt from the records when Scribe starts. - **Search indexes.** They are rebuilt from the records when Scribe starts.
- **Sharing:** groups, project shares and note shares. - **Sharing:** groups, project shares and note shares.
- **Credentials:** API keys and forge connections. After a restore, issue new - **Credentials:** API keys and forge connections. After a restore, issue new
API keys and add forge connections again. API keys and add forge connections again. (Forge tokens are also sealed under
`SECRET_KEY` in the database itself, so a copy of the database doesn't carry
them in readable form either.)
- **Logs and notifications.** - **Logs and notifications.**
- **The backup destination's own S3 secret.** - **The backup destination's own S3 secret.**
+2 -1
View File
@@ -9,7 +9,7 @@ Configuration is via environment variables. The `docker-compose.yml` file sets d
| Variable | Default | Description | | Variable | Default | Description |
|----------|---------|-------------| |----------|---------|-------------|
| `DATABASE_URL` | `postgresql+asyncpg://fabled:fabled@db/scribe` | PostgreSQL async connection string | | `DATABASE_URL` | `postgresql+asyncpg://fabled:fabled@db/scribe` | PostgreSQL async connection string |
| `SECRET_KEY` | `dev-secret-change-me` | Session signing key — **change this in production** | | `SECRET_KEY` | `dev-secret-change-me` | Session signing key, and the key stored credentials are sealed under — **change this in production, then keep it** |
| `SECRET_KEY_FILE` | — | Path to a Docker secret file containing the key (alternative to `SECRET_KEY`) | | `SECRET_KEY_FILE` | — | Path to a Docker secret file containing the key (alternative to `SECRET_KEY`) |
| `LOG_LEVEL` | `INFO` | Logging verbosity (`DEBUG`, `INFO`, `WARNING`, `ERROR`) | | `LOG_LEVEL` | `INFO` | Logging verbosity (`DEBUG`, `INFO`, `WARNING`, `ERROR`) |
| `SECURE_COOKIES` | `false` | Set `true` when running behind TLS | | `SECURE_COOKIES` | `false` | Set `true` when running behind TLS |
@@ -127,6 +127,7 @@ If using `TRUST_PROXY_HEADERS=true`, ensure your proxy strips any client-supplie
python3 -c "import secrets; print(secrets.token_hex(32))" python3 -c "import secrets; print(secrets.token_hex(32))"
``` ```
Or use Docker Secrets via `SECRET_KEY_FILE`. Or use Docker Secrets via `SECRET_KEY_FILE`.
- **Keep `SECRET_KEY` once it is set.** Scribe seals the credentials it stores under a key derived from it: forge-connection tokens and the backup destination's S3 secret. A copy of the database alone can't read them. If the key changes (for example, a restore onto an instance with a different key), those credentials can no longer be opened. Settings marks each one and asks for it again, and nothing else breaks.
- **`SECURE_COOKIES=true`** — must be set when running behind TLS. - **`SECURE_COOKIES=true`** — must be set when running behind TLS.
- **`BASE_URL`** — set to your public URL; required for OIDC redirects and email links. - **`BASE_URL`** — set to your public URL; required for OIDC redirects and email links.
- **Registration** — auto-closes after the first user (admin). Re-enable from Settings → Users or send invite links. - **Registration** — auto-closes after the first user (admin). Re-enable from Settings → Users or send invite links.
+21 -3
View File
@@ -825,14 +825,17 @@ const baseUrlSaved = ref(false);
// Git forge connections (#2778) — the user's keyring: one read-only // Git forge connections (#2778) — the user's keyring: one read-only
// credential per forge host, used server-side for every forge read on // credential per forge host, used server-side for every forge read on
// projects this user owns. The token round-trips masked; the server treats // projects this user owns. The token round-trips masked; the server treats
// the mask as "unchanged". // the mask as "unchanged". Tokens are stored sealed under SECRET_KEY:
// `token_readable` false means this instance can no longer open one (the key
// changed since it was saved), so the row asks for it again.
interface ForgeConnectionEntry { interface ForgeConnectionEntry {
id: number; kind: string; base_url: string; host: string; id: number; kind: string; base_url: string; host: string; token_readable: boolean;
} }
const forgeConnections = ref<ForgeConnectionEntry[]>([]); const forgeConnections = ref<ForgeConnectionEntry[]>([]);
const forgeKinds = ref<string[]>(["gitea", "github"]); const forgeKinds = ref<string[]>(["gitea", "github"]);
const connForm = ref({ id: 0, kind: "gitea", base_url: "", token: "" }); const connForm = ref({ id: 0, kind: "gitea", base_url: "", token: "" });
const connFormOpen = ref(false); const connFormOpen = ref(false);
const unreadableConnections = computed(() => forgeConnections.value.filter((c) => !c.token_readable));
const savingConn = ref(false); const savingConn = ref(false);
const testingConnId = ref(0); const testingConnId = ref(0);
const connTestResult = ref<{ id: number; ok: boolean; message: string } | null>(null); const connTestResult = ref<{ id: number; ok: boolean; message: string } | null>(null);
@@ -2564,13 +2567,23 @@ async function deleteUser(userId: number) {
Read-only, base URL <code>https://github.com</code> (or your GitHub Read-only, base URL <code>https://github.com</code> (or your GitHub
Enterprise URL). Enterprise URL).
</p> </p>
<p v-if="unreadableConnections.length" class="calibration-warning" role="alert">
{{ unreadableConnections.length === 1 ? "One connection's token" : `${unreadableConnections.length} connections' tokens` }}
can't be read on this instance: <code>SECRET_KEY</code> has changed since
{{ unreadableConnections.length === 1 ? "it was" : "they were" }} saved. Edit
{{ unreadableConnections.map((c) => c.host).join(", ") }} and enter the token again.
Until then, forge reads for that host don't run.
</p>
<table v-if="forgeConnections.length" class="api-keys-table"> <table v-if="forgeConnections.length" class="api-keys-table">
<thead> <thead>
<tr><th>Host</th><th>Kind</th><th>Base URL</th><th></th></tr> <tr><th>Host</th><th>Kind</th><th>Base URL</th><th></th></tr>
</thead> </thead>
<tbody> <tbody>
<tr v-for="c in forgeConnections" :key="c.id"> <tr v-for="c in forgeConnections" :key="c.id">
<td>{{ c.host }}</td> <td>
{{ c.host }}
<span v-if="!c.token_readable" class="conn-token-lost">token needs re-entering</span>
</td>
<td>{{ c.kind }}</td> <td>{{ c.kind }}</td>
<td><code>{{ c.base_url }}</code></td> <td><code>{{ c.base_url }}</code></td>
<td> <td>
@@ -4593,6 +4606,11 @@ async function deleteUser(userId: number) {
border-radius: var(--fs-radius-md); border-radius: var(--fs-radius-md);
background: color-mix(in srgb, var(--fs-warning) 12%, var(--fs-surface-raised)); background: color-mix(in srgb, var(--fs-warning) 12%, var(--fs-surface-raised));
} }
.conn-token-lost {
margin-left: var(--fs-space-2);
font-size: var(--fs-size-body-sm);
color: var(--fs-warning-fg);
}
.calibration-warning-title { .calibration-warning-title {
margin: 0 0 var(--fs-space-2); margin: 0 0 var(--fs-space-2);
font-size: 0.95rem; font-size: 0.95rem;
+4 -2
View File
@@ -16,7 +16,9 @@ class ForgeConnection(Base, TimestampMixin):
`host` is derived from `base_url` at write time and stored because it is `host` is derived from `base_url` at write time and stored because it is
the lookup key; the service layer keeps the two in step. The token is a the lookup key; the service layer keeps the two in step. The token is a
secret: to_dict never includes it, and no route may return it. secret: it is stored sealed (services/secrets.py — read it through
services.forge_connections.connection_token), to_dict never includes it,
and no route may return it.
""" """
__tablename__ = "forge_connections" __tablename__ = "forge_connections"
@@ -31,7 +33,7 @@ class ForgeConnection(Base, TimestampMixin):
kind: Mapped[str] = mapped_column(Text, nullable=False) kind: Mapped[str] = mapped_column(Text, nullable=False)
base_url: Mapped[str] = mapped_column(Text, nullable=False) base_url: Mapped[str] = mapped_column(Text, nullable=False)
host: Mapped[str] = mapped_column(Text, nullable=False) host: Mapped[str] = mapped_column(Text, nullable=False)
token: Mapped[str] = mapped_column(Text, nullable=False) token_sealed: Mapped[str] = mapped_column(Text, nullable=False)
def to_dict(self) -> dict: def to_dict(self) -> dict:
return { return {
+15 -4
View File
@@ -125,12 +125,17 @@ async def test_search():
@login_required @login_required
async def list_forge_connections_route(): async def list_forge_connections_route():
from scribe.services.forge import FORGE_KINDS from scribe.services.forge import FORGE_KINDS
from scribe.services.forge_connections import list_connections from scribe.services.forge_connections import connection_token, list_connections
uid = get_current_user_id() uid = get_current_user_id()
rows = await list_connections(uid) rows = await list_connections(uid)
# Never the token: only whether this instance can still open it, so the
# page can ask for it again after a SECRET_KEY change.
return jsonify({ return jsonify({
"connections": [r.to_dict() for r in rows], "connections": [
{**r.to_dict(), "token_readable": connection_token(r) is not None}
for r in rows
],
"kinds": list(FORGE_KINDS), "kinds": list(FORGE_KINDS),
}) })
@@ -198,13 +203,19 @@ async def test_forge_connection_route(connection_id: int):
press, so a misconfiguration is visible now rather than as silent press, so a misconfiguration is visible now rather than as silent
fallbacks later (#2663's lesson, applied per keyring row).""" fallbacks later (#2663's lesson, applied per keyring row)."""
from scribe.services.forge import ForgeError, build_adapter from scribe.services.forge import ForgeError, build_adapter
from scribe.services.forge_connections import get_connection from scribe.services.forge_connections import connection_token, get_connection
uid = get_current_user_id() uid = get_current_user_id()
row = await get_connection(uid, connection_id) row = await get_connection(uid, connection_id)
if row is None: if row is None:
return jsonify({"error": "Connection not found"}), 404 return jsonify({"error": "Connection not found"}), 404
adapter = build_adapter(row.kind, row.base_url, row.token) token = connection_token(row)
if token is None:
return jsonify({
"error": "This instance can't read the saved token: SECRET_KEY has changed "
"since it was saved. Enter the token again."
}), 400
adapter = build_adapter(row.kind, row.base_url, token)
if adapter is None: if adapter is None:
return jsonify({"error": "Connection is not usable — check kind and base URL"}), 400 return jsonify({"error": "Connection is not usable — check kind and base URL"}), 400
try: try:
+18 -2
View File
@@ -358,6 +358,22 @@ _FORGE_CLASSES: dict[str, type[ForgeAdapter]] = {
} }
def _token_of(row) -> str:
"""A keyring row's token, opened; empty when this instance can't open it,
which build_adapter reads as "this connection cannot serve reads"."""
from scribe.services.forge_connections import connection_token
token = connection_token(row)
if token is None:
logger.warning(
"forge connection %s (%s) holds a token this instance can't open — "
"SECRET_KEY changed since it was saved; its owner must enter it again",
row.id, row.host,
)
return ""
return token
def build_adapter( def build_adapter(
kind: str, base_url: str, token: str, *, transport=None kind: str, base_url: str, token: str, *, transport=None
) -> ForgeAdapter | None: ) -> ForgeAdapter | None:
@@ -459,7 +475,7 @@ async def get_forges(
if pinned_id: if pinned_id:
pin = next((r for r in rows if r.id == pinned_id), None) pin = next((r for r in rows if r.id == pinned_id), None)
if pin is not None: if pin is not None:
adapter = build_adapter(pin.kind, pin.base_url, pin.token, transport=transport) adapter = build_adapter(pin.kind, pin.base_url, _token_of(pin), transport=transport)
return ForgeSelector((adapter,) if adapter is not None else ()) return ForgeSelector((adapter,) if adapter is not None else ())
logger.warning( logger.warning(
"project %s pins forge connection %s the owner (%s) does not hold — pin ignored", "project %s pins forge connection %s the owner (%s) does not hold — pin ignored",
@@ -468,7 +484,7 @@ async def get_forges(
adapters: list[ForgeAdapter] = [] adapters: list[ForgeAdapter] = []
for row in rows: for row in rows:
adapter = build_adapter(row.kind, row.base_url, row.token, transport=transport) adapter = build_adapter(row.kind, row.base_url, _token_of(row), transport=transport)
if adapter is not None: if adapter is not None:
adapters.append(adapter) adapters.append(adapter)
if role == "admin": if role == "admin":
+14 -3
View File
@@ -4,7 +4,9 @@ A connection is a user's read-only credential for one forge host; one row per
(user, host) keeps host-keyed resolution deterministic with no default-pointer (user, host) keeps host-keyed resolution deterministic with no default-pointer
machinery. Everything here is own-rows-only: a connection is a credential, and machinery. Everything here is own-rows-only: a connection is a credential, and
no caller — admin included — reads or edits another user's. The token never no caller — admin included — reads or edits another user's. The token never
leaves the server (model.to_dict omits it; routes mask "set/unset"). leaves the server (model.to_dict omits it; routes mask "set/unset"), and it is
stored sealed (services/secrets.py): a copy of the database alone cannot read
it. Every reader opens it through `connection_token`.
Validation matches what build_adapter will accept, checked here so a bad Validation matches what build_adapter will accept, checked here so a bad
value is a 400 at the form instead of a silently dead keyring row. value is a 400 at the form instead of a silently dead keyring row.
@@ -17,6 +19,14 @@ from scribe.models import async_session
from scribe.models.forge_connection import ForgeConnection from scribe.models.forge_connection import ForgeConnection
from scribe.models.project import Project from scribe.models.project import Project
from scribe.services.forge import FORGE_KINDS, host_of from scribe.services.forge import FORGE_KINDS, host_of
from scribe.services.secrets import seal, unseal
def connection_token(row: ForgeConnection) -> str | None:
"""The connection's token in the clear, or None when this instance can't
open it — sealed under a different SECRET_KEY (a restore onto a new
instance, a rotated key). The owner enters it again; nothing else can."""
return unseal(row.token_sealed)
def validate_connection(kind: str, base_url: str) -> str | None: def validate_connection(kind: str, base_url: str) -> str | None:
@@ -82,7 +92,8 @@ async def create_connection(
"resolution is by host, so a second row could never be reached" "resolution is by host, so a second row could never be reached"
) )
row = ForgeConnection( row = ForgeConnection(
user_id=user_id, kind=kind, base_url=base_url, host=host, token=token user_id=user_id, kind=kind, base_url=base_url, host=host,
token_sealed=seal(token),
) )
session.add(row) session.add(row)
await session.commit() await session.commit()
@@ -135,7 +146,7 @@ async def update_connection(
row.base_url = new_base row.base_url = new_base
row.host = new_host row.host = new_host
if token: if token:
row.token = token row.token_sealed = seal(token)
await session.commit() await session.commit()
await session.refresh(row) await session.refresh(row)
return row return row
+47
View File
@@ -0,0 +1,47 @@
"""Forge-connection tokens are stored sealed (milestone 523, #5877).
Migration 0122 seals the rows that existed before, with its OWN frozen copy of
the v1 key derivation (so a later edit to services/secrets.py can never change
what an old migration writes). That copy is only right while it agrees with
what the app opens today — which is what this pins. If secrets.py ever moves
to a v2 key, it must still open v1 values, and this test says so.
"""
import importlib.util
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import patch
from scribe.services import secrets
from scribe.services.forge_connections import connection_token
_MIGRATION = Path(__file__).resolve().parents[1] / "alembic" / "versions" / "0122_seal_forge_tokens.py"
def _migration():
spec = importlib.util.spec_from_file_location("migration_0122", _MIGRATION)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module
def test_what_the_migration_seals_the_app_opens():
sealed = _migration().seal_v1("gitea-read-token")
assert "gitea-read-token" not in sealed
assert secrets.unseal(sealed) == "gitea-read-token"
def test_a_row_reads_through_connection_token():
row = SimpleNamespace(token_sealed=secrets.seal("ghp_abc"))
assert connection_token(row) == "ghp_abc"
def test_a_row_sealed_under_another_key_reads_as_none():
row = SimpleNamespace(token_sealed=secrets.seal("ghp_abc"))
with patch.object(secrets.Config, "SECRET_KEY", "a-different-instance"):
assert connection_token(row) is None
def test_a_left_over_plain_text_token_is_not_used_as_one():
"""A value that never went through seal is refused, not passed to the
forge as if it were the token: a reader sees None, never the raw bytes."""
assert connection_token(SimpleNamespace(token_sealed="plain-text-token")) is None
+26
View File
@@ -169,3 +169,29 @@ async def test_env_config_serves_admins_only_and_rows_beat_it(seeded):
assert len(selector.adapters) == 1 assert len(selector.adapters) == 1
assert selector.adapters[0]._token == "row-tok" assert selector.adapters[0]._token == "row-tok"
await delete_connection(admin, row.id) await delete_connection(admin, row.id)
@pytest.mark.integration
async def test_the_token_is_stored_sealed_and_a_key_change_drops_the_row(seeded):
"""What a copy of the database holds is ciphertext; the reads still get
the token; and a row this instance can no longer open serves nothing
rather than sending ciphertext to the forge as a token."""
from sqlalchemy import text
from scribe.services import secrets
owner = seeded["owner"]
row = await create_connection(owner, kind="gitea", base_url=GITEA, token="plain-tok")
await update_connection(owner, row.id, token="rotated-tok")
async with async_session() as s:
stored = (await s.execute(
text("SELECT token_sealed FROM forge_connections WHERE id = :id"), {"id": row.id}
)).scalar_one()
assert "rotated-tok" not in stored
assert secrets.unseal(stored) == "rotated-tok"
selector = await get_forges(owner, seeded["project"])
assert [a._token for a in selector.adapters] == ["rotated-tok"]
with patch.object(secrets.Config, "SECRET_KEY", "a-different-instance"):
assert not (await get_forges(owner, seeded["project"])).configured