Add SSH key generation for callis↔client and callis↔host pairing by Copilot · Pull Request #21 · pacnpal/callis

Copilot · 2026-04-07T15:10:43Z

Remove unused generated_key_page.html template — the non-HTMX generate-key path now returns HTTP 400 instead of rendering this full page, making the template dead code

…iring Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/4a470c3f-5186-4816-849d-aa0ae0179751 Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot

Pull request overview

Adds server-side Ed25519 keypair generation to support two SSH pairing flows in Callis: user self-service key generation (show private key once) and an instance-scoped “server deploy key” for admins to install on target hosts.

Changes:

Added Ed25519 keypair generation utilities and a deploy-key accessor in api/core.py.
Added POST /users/{user_id}/keys/generate plus UI/partial to display the generated private key once and refresh the key list.
Exposed the server deploy public key on the Hosts page and added client-side support to download the generated private key.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
api/core.py	Adds SSH keypair generation and a persisted server deploy key retrieval function.
api/routers/users.py	Adds endpoint to generate a user SSH key and return the private key once in an htmx partial.
api/routers/hosts.py	Passes server deploy public key into the Hosts template context.
api/templates/user_detail.html	Adds “Generate Key” dialog flow for user SSH keys.
api/templates/partials/generated_key.html	New partial that displays the generated private key once + OOB key list refresh.
api/templates/hosts.html	Adds admin-only “Callis Server Key” section showing the deploy public key and install command.
api/static/app.js	Adds click handler to download key text content as a file.

Copilot · 2026-04-07T15:16:02Z

+    # Private key exists but public key file is missing — derive it.
+    try:
+        with open(priv_path, "rb") as f:
+            priv_bytes = f.read()
+        priv = load_pem_private_key(priv_bytes, password=None)
+        pub_text = priv.public_key().public_bytes(
+            encoding=Encoding.OpenSSH,
+            format=PublicFormat.OpenSSH,
+        ).decode().strip()


get_server_deploy_public_key() writes the private key using PrivateFormat.OpenSSH, but later tries to reload it with load_pem_private_key(). load_pem_private_key does not parse OpenSSH private-key format ("BEGIN OPENSSH PRIVATE KEY"), so this path will fail and can prevent recovering the persisted key. Use serialization.load_ssh_private_key() (or persist the key in a PEM format that load_pem_private_key supports, e.g. PKCS8) so the derived-public-key path works reliably.

Copilot · 2026-04-07T15:16:03Z

+        # Another concurrent call beat us — recurse once to pick up the new key.
+        return get_server_deploy_public_key()


On FileExistsError this function recurses without a cap. If the private key file exists but is unreadable/unparseable (or the public key file can’t be written), this can recurse indefinitely and potentially stack overflow. Consider replacing recursion with a single retry that re-reads the .pub (or returns the in-memory public_key_text / empty string) and/or add a bounded retry counter.

Suggested change

# Another concurrent call beat us — recurse once to pick up the new key.

return get_server_deploy_public_key()

# Another concurrent call beat us. Re-read the created key material once

# instead of recursing indefinitely if the files are unreadable or incomplete.

try:

with open(pub_path) as f:

return f.read().strip()

except FileNotFoundError:

pass

except OSError as exc:

logger.warning("Could not read deploy public key at %s after concurrent creation: %s", pub_path, exc)

try:

with open(priv_path, "rb") as f:

priv_bytes = f.read()

priv = load_pem_private_key(priv_bytes, password=None)

return priv.public_key().public_bytes(

encoding=Encoding.OpenSSH,

format=PublicFormat.OpenSSH,

).decode().strip()

except FileNotFoundError:

logger.warning(

"Deploy private key at %s disappeared after concurrent creation; "

"returning generated public key without persisting it.",

priv_path,

)

except Exception as exc:

logger.warning(

"Could not load existing deploy private key at %s after concurrent creation: %s",

priv_path,

exc,

)

return public_key_text

Copilot · 2026-04-07T15:16:03Z

+def get_server_deploy_public_key() -> str:
+    """Return Callis's server deploy public key, generating it if needed.
+
+    The keypair is persisted to /data/callis_deploy_key[.pub].  Returns the
+    OpenSSH public key as a single-line string, or an empty string if the key
+    cannot be generated (e.g. /data is not writable in dev without Docker).
+
+    This function performs synchronous file I/O.  It is intended to be called
+    at most once per process lifetime (on first access) so the blocking impact
+    is negligible.
+    """
+    priv_path = _DEPLOY_KEY_PATH
+    pub_path = priv_path + ".pub"
+
+    # Fast path: public key file already exists.
+    try:
+        with open(pub_path) as f:
+            return f.read().strip()
+    except FileNotFoundError:


This function is documented as "intended to be called at most once per process lifetime", but it performs synchronous disk I/O every time it’s invoked (at least a open().read() on the .pub). Consider caching the result (e.g. @lru_cache(maxsize=1) or a module-level memo) after the first successful read/generation to avoid blocking the event loop on each request.

Copilot · 2026-04-07T15:16:03Z

+        (private_key_openssh: str, public_key_openssh: str)
+
+    The private key is in OpenSSH PEM format.  The public key is a single
+    authorised-keys line; an optional comment is appended when provided.


Docstring wording: "authorised-keys" should be "authorized_keys" (consistent with the rest of the codebase and the actual OpenSSH filename).

Suggested change

authorised-keys line; an optional comment is appended when provided.

authorized_keys line; an optional comment is appended when provided.

Copilot · 2026-04-07T15:16:04Z

    # Load all active users for assignment dropdowns (admin only)
    all_users = []
    if user.role == UserRole.admin:
        users_result = await db.execute(
            select(User).where(User.is_active == True).order_by(User.username)
        )
        all_users = users_result.scalars().all()

    return templates.TemplateResponse(
        request,
        "hosts.html",
-        context={"hosts": hosts, "user": user, "settings": settings, "ssh_host": ssh_host, "all_users": all_users},
+        context={"hosts": hosts, "user": user, "settings": settings, "ssh_host": ssh_host, "all_users": all_users, "server_deploy_key": get_server_deploy_public_key()},
    )


get_server_deploy_public_key() does synchronous file I/O and may generate/persist a new key. It’s currently called on every /hosts request (including for non-admin users), which can block the event loop and can create the deploy key before an admin ever visits the page. Consider only calling it when user.role == admin and otherwise passing an empty string/None.

Copilot · 2026-04-07T15:16:04Z

    async def _form_error(detail: str):
        settings = get_settings()
        ssh_host = urlparse(await get_runtime_setting("base_url")).hostname or "localhost"
        result = await db.execute(
            select(Host).options(selectinload(Host.assigned_users)).order_by(Host.created_at.desc())
        )
        all_hosts = result.scalars().all()
        au = []
        if user.role == UserRole.admin:
            ur = await db.execute(select(User).where(User.is_active == True).order_by(User.username))
            au = ur.scalars().all()
        return templates.TemplateResponse(
            request,
            "hosts.html",
-            context={"error": detail, "hosts": all_hosts, "user": user, "settings": settings, "ssh_host": ssh_host, "all_users": au},
+            context={"error": detail, "hosts": all_hosts, "user": user, "settings": settings, "ssh_host": ssh_host, "all_users": au, "server_deploy_key": get_server_deploy_public_key()},
            status_code=400,


Same as above: _form_error recomputes server_deploy_key (sync disk I/O) on error responses. If the value is cached/memoized or only computed for admins, this path should follow the same approach to avoid repeated blocking work during form validation errors.

Copilot · 2026-04-07T15:16:04Z

+    return templates.TemplateResponse(
+        request,
+        "partials/generated_key.html",
+        context={
+            "private_key": private_key_text,
+            "label": label,
+            "fingerprint": key_info["fingerprint"],
+            "keys": keys,
+            "target_user_id": user_id,
+            "user": user,
+        },
+    )


This endpoint returns a newly generated private key in the response body. To reduce the chance of it being stored by browser/proxy caches, consider setting Cache-Control: no-store (and optionally Pragma: no-cache) on this TemplateResponse (or in a shared middleware for this route).

pacnpal · 2026-04-07T18:54:02Z

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/35cc0b82-8cb9-4495-9082-69daac104a0d Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-07T18:59:34Z

@copilot apply changes based on the comments in this thread

Applied all review feedback in cc846e7:

load_pem_private_key → load_ssh_private_key — OpenSSH private key format ("BEGIN OPENSSH PRIVATE KEY") requires the SSH-specific loader; the PEM loader doesn't handle it.
Docstring typo — "authorised-keys" → "authorized_keys".
Result caching — Added module-level _deploy_public_key_cache; get_server_deploy_public_key() now does disk I/O at most once per process lifetime.
Unbounded recursion on FileExistsError — Replaced with a bounded single retry (read .pub, then re-derive from the private key file); returns "" as a last resort so we never return a public key that doesn't match what's on disk.
Duplicate private-key-loading logic — Extracted into _derive_public_key_from_private_file(priv_path, pub_path) helper used in both the "recover from missing .pub" path and the concurrent-write retry.
get_server_deploy_public_key() called for non-admins — Now only called inside the if user.role == UserRole.admin block in both host_list and _form_error.
Cache-Control: no-store — Added Cache-Control: no-store and Pragma: no-cache headers to the POST /users/{id}/keys/generate response.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-07T19:42:34Z

+            "A new key will be generated on next restart.",
+            priv_path,
+            exc,
+        )
+        return public_key_text


get_server_deploy_public_key() documents returning an empty string when the deploy key cannot be generated (e.g. /data not writable), but on PermissionError/OSError while persisting the private key it returns the newly generated public_key_text. That public key will be unusable after a restart (since the private key wasn’t saved), and it can lead admins to install a deploy key that silently breaks later. Consider returning an empty string in this case (or only returning a key once persistence succeeds) to match the docstring and avoid operational surprises.

Suggested change

"A new key will be generated on next restart.",

priv_path,

exc,

)

return public_key_text

"Returning empty string because the generated key is not durable.",

priv_path,

exc,

)

return ""

Copilot · 2026-04-07T19:42:35Z

 <div class="page-header">
  <h3>SSH Keys</h3>
+  <button type="button" class="outline btn-sm" data-dialog-open="generate-key-dialog">Generate Key</button>


The "Generate Key" action is shown unconditionally, but the backend rejects key generation for inactive users. Hiding or disabling this button when target_user.is_active is false would prevent a user-visible failure path in the UI (and matches the existing “Status” indicator above).

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

pacnpal · 2026-04-07T19:43:11Z

@copilot apply changes based on the comments in this thread

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-07T19:47:29Z

+_deploy_public_key_cache: str | None = None
+
+


_deploy_public_key_cache is declared twice at module scope. This is redundant and makes it easier to accidentally reinitialize the cache later; keep a single definition.

Suggested change

_deploy_public_key_cache: str | None = None

Copilot · 2026-04-07T19:47:30Z

+    except FileExistsError:
+        # Another concurrent call beat us.  Try to read the key they persisted;
+        # return empty string rather than a mismatched in-memory key if that fails.
+        try:
+            with open(pub_path) as f:
+                return f.read().strip()
+        except FileNotFoundError:
+            pass
+        except OSError as exc:
+            logger.warning(
+                "Could not read deploy public key at %s after concurrent creation: %s",
+                pub_path, exc,
+            )
+
+        derived = _derive_public_key_from_private_file(priv_path, pub_path)
+        if derived is not None:
+            return derived
+


In the FileExistsError recovery path, successful reads/derivations (open(pub_path) / _derive_public_key_from_private_file) are returned without updating _deploy_public_key_cache. That defeats the “cached after first successful read” guarantee and can cause repeated disk I/O on subsequent calls; assign the cache before returning in these branches.

Copilot · 2026-04-07T19:47:30Z

+    return templates.TemplateResponse(
+        request,
+        "partials/generated_key.html",
+        context={
+            "private_key": private_key_text,
+            "label": label,
+            "fingerprint": key_info["fingerprint"],
+            "keys": keys,
+            "target_user_id": user_id,
+            "user": user,
+        },
+        headers={"Cache-Control": "no-store", "Pragma": "no-cache"},
+    )


This endpoint always returns the partials/generated_key.html fragment, even for non-htmx requests. In the non-htmx case the browser will navigate to a fragment-only page (no base layout/assets), which is inconsistent with the existing /users/{user_id}/keys behavior (redirect on non-HX). Consider branching on HX-Request and either (a) render a full page that includes the generated private key, or (b) return a redirect/error explaining that key generation requires the web UI dialog.

Copilot · 2026-04-07T19:47:30Z

+def _derive_public_key_from_private_file(priv_path: str, pub_path: str) -> str | None:
+    """Load the deploy private key from *priv_path* and return its OpenSSH public key.
+
+    Writes the derived public key to *pub_path* as a side-effect when possible.
+    Returns ``None`` (and logs a warning) if the file is missing or unreadable.
+    """
+    try:
+        with open(priv_path, "rb") as f:
+            priv_bytes = f.read()
+        priv = load_ssh_private_key(priv_bytes, password=None)
+        pub_text = priv.public_key().public_bytes(
+            encoding=Encoding.OpenSSH,
+            format=PublicFormat.OpenSSH,
+        ).decode().strip()
+        try:
+            with open(pub_path, "w") as fh:
+                fh.write(pub_text + "\n")
+        except OSError as exc:
+            logger.warning("Could not write deploy public key to %s: %s", pub_path, exc)
+        return pub_text
+    except FileNotFoundError:
+        return None
+    except Exception as exc:
+        logger.warning("Could not load deploy private key at %s: %s", priv_path, exc)
+        return None


Docstring says _derive_public_key_from_private_file() returns None “(and logs a warning) if the file is missing”, but the FileNotFoundError branch returns None without logging. Either adjust the docstring or log in that branch for consistency.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-08T15:52:19Z

+                f'<p class="text-error" role="alert">{html.escape(exc.detail)}</p>',
+                status_code=200,
+                headers={
+                    "HX-Retarget": "#generate-key-error",


HX-Retarget is set to #generate-key-error for HTMX label validation failures, but there is no element with that id in the dialog markup. This means the error fragment won’t be displayed.

Add an element (e.g. a dedicated error container) with id="generate-key-error" in the dialog, or change the retarget header to point at an existing element in the generate-key dialog.

Suggested change

"HX-Retarget": "#generate-key-error",

Copilot · 2026-04-08T15:52:19Z

    # Verify target user exists and is active
    target_result = await db.execute(select(User).where(User.id == user_id))
    target = target_result.scalar_one_or_none()
    if not target:
        raise HTTPException(status_code=404, detail="User not found")
    if not target.is_active:
        raise HTTPException(status_code=400, detail="Cannot upload keys for inactive user")

    # Check key limit
-    max_keys = await get_runtime_setting("max_keys_per_user")
-    count_result = await db.execute(
-        select(func.count()).where(SSHKey.user_id == user_id, SSHKey.is_active == True)
-    )
-    current_count = count_result.scalar()
-    if current_count >= max_keys:
-        raise HTTPException(
-            status_code=400,
-            detail=f"Maximum {max_keys} keys per user",
-        )
+    await _check_key_limit(user_id, db)
+
+    # Validate label
+    try:
+        label = _validate_label(label)
+        if not label:
+            raise HTTPException(status_code=400, detail="Label cannot be blank")
+    except HTTPException as exc:
+        if request.headers.get("HX-Request"):
+            return HTMLResponse(
+                f'<span class="text-error">{html.escape(exc.detail)}</span>',
+                status_code=200,
+                headers={"HX-Retarget": "#upload-error-msg", "HX-Reswap": "innerHTML"},
+            )
+        raise


upload_key() is submitted via HTMX (hx-post in user_detail.html), but only label validation errors are converted into a 200 + fragment response. Other user-facing validation errors (invalid public key, duplicate fingerprint, key limit, inactive user, etc.) still raise HTTPException(400), which results in no swap and a poor UX for the in-page form.

Consider catching these HTTPExceptions when HX-Request is present and returning an error fragment retargeted to #upload-error-msg (and similarly for generate-key) so the user gets feedback without a full-page error response.

pacnpal · 2026-04-08T15:52:25Z

@copilot apply changes based on the comments in this thread

…MX error handling to all 400 cases Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/810c2d20-be06-4e0f-a41b-0bc4954ba0a3 Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-08T15:58:08Z

@copilot apply changes based on the comments in this thread

Applied both items from review #4076444569 in 9afc404:

Missing #generate-key-error element (user_detail.html) — added <p id="generate-key-error" aria-live="polite" role="alert" class="text-error"></p> inside the form in the generate-key dialog (above the submit button). The HX-Retarget: #generate-key-error from generate_key() now has a valid target; errors are shown inline while the form stays intact for resubmission.
HTMX error handling coverage (users.py) — for both upload_key() and generate_key(), the inactive-user check, key-limit check, label validation, key-parse error, and duplicate-fingerprint check are now all inside a single try/except HTTPException block. Any 400 error on an HTMX request returns a 200 HTML fragment retargeted to #upload-error-msg / #generate-key-error instead of letting the global handler render a full 500.html that would break the page layout. 404 errors (user not found) are kept outside the try block and still return full-page responses.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-08T16:03:08Z

+    # Generate Ed25519 keypair; use username as the key comment
+    private_key_text, public_key_text = generate_ssh_keypair(comment=target.username)
+
+    try:
+        key_info = parse_ssh_public_key(public_key_text)
+    except ValueError as e:
+        safe_user_id = (user_id or "").replace("\r", "").replace("\n", "")
+        logger.exception("Key generation internal error for user %s: %s", safe_user_id, e)
+        raise HTTPException(status_code=500, detail="Key generation failed")
+
+    new_key = SSHKey(
+        user_id=user_id,
+        label=label,
+        public_key_text=key_info["public_key_text"],
+        fingerprint=key_info["fingerprint"],
+        key_type=key_info["key_type"],
+    )
+    db.add(new_key)
+    await db.flush()


generate_key() doesn't check for an existing active key with the same fingerprint before inserting. upload_key() does this and the PR description mentions handling duplicate fingerprints; without the check you can still end up with duplicate keys for a user (or unexpected behavior if uniqueness is later enforced). Add the same duplicate-fingerprint query/HTTP 400 path here (and keep the HTMX-friendly error response).

Copilot · 2026-04-08T16:03:08Z

+  if (genDialog && genKeyBody) {
+    var _genKeyBodyInitial = genKeyBody.innerHTML;
+    genDialog.addEventListener("close", function () {
+      genKeyBody.innerHTML = _genKeyBodyInitial;


The dialog reset logic replaces #generate-key-body using innerHTML, which recreates the form DOM nodes. htmx won't automatically bind to elements inserted by custom JS, so after the first close/reopen the restored form may stop performing hx-post/hx-target and fall back to a full page submit. After restoring, call window.htmx.process(genKeyBody) (guarded by if (window.htmx)) to reinitialize htmx behavior.

Suggested change

genKeyBody.innerHTML = _genKeyBodyInitial;

genKeyBody.innerHTML = _genKeyBodyInitial;

if (window.htmx) {

window.htmx.process(genKeyBody);

}

Copilot · 2026-04-08T16:03:09Z

+            logger.warning(
+                "Could not recover deploy public key after concurrent creation at %s; "
+                "returning empty string.",
+                priv_path,
+            )
+            return ""


The docstring says failures are cached as an empty string, but this branch returns "" without setting _deploy_public_key_cache. That means repeated calls (e.g. on every /hosts page render) will retry the same blocking disk I/O and logging instead of fast-returning. Set _deploy_public_key_cache = "" before returning so failures are actually cached as described.

Copilot · 2026-04-08T16:03:09Z

+        except (PermissionError, OSError) as exc:
+            logger.warning(
+                "Could not persist server deploy key to %s: %s. "
+                "Returning empty string because the generated key is not durable.",
+                priv_path,
+                exc,
+            )
+            return ""


Same caching issue here: on persistence errors this returns "" but leaves _deploy_public_key_cache as None, so callers will keep redoing the expensive/blocked work on every invocation. Cache the failure (_deploy_public_key_cache = "") before returning to match the docstring and avoid repeated I/O in request handlers.

pacnpal · 2026-04-08T16:51:03Z

@copilot apply changes based on the comments in this thread

…n reset, cache failure in both core.py branches Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/463cd7b9-b951-4b02-84bc-9eb45711b06e Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-08T16:55:13Z

@copilot apply changes based on the comments in this thread

Applied all four items from review #4076509145 in 346b012:

Duplicate fingerprint check in generate_key() (users.py) — after parsing the generated public key, queries for an active key with the same fingerprint. HTMX requests return a 200 error fragment retargeted to #generate-key-error; non-HTMX raises HTTP 400.
window.htmx.process() after dialog reset (app.js) — calls window.htmx.process(genKeyBody) (guarded by if (window.htmx)) after restoring the form's innerHTML on dialog close, so htmx bindings remain active on reopen.
Cache failures in core.py — both the FileExistsError recovery branch (line 710) and the PermissionError/OSError persistence branch (line 719) now set _deploy_public_key_cache = "" before returning, so subsequent calls fast-return instead of retrying the same disk I/O. The global _deploy_public_key_cache declaration at the top of the function (line 613) applies to both assignments.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-08T17:46:53Z

+  {% if target_user.is_active %}
+  <button type="button" class="outline btn-sm" data-dialog-open="generate-key-dialog">Generate Key</button>
+  {% endif %}
 </div>


When target_user is inactive, the UI hides the “Generate Key” button, but the “Upload Key” form is still shown and will always fail server-side (Cannot upload keys for inactive user). Consider also hiding/disabled the upload form (or disabling its submit) when the user is inactive to avoid a confusing error-only flow.

Copilot · 2026-04-08T17:46:53Z

+        except FileExistsError:
+            # File was created between our check and the open call (e.g. by another
+            # process on first startup). Try to read the key they persisted; fall
+            # back to deriving it from the private key file.
+            try:
+                with open(pub_path) as f:
+                    first_line = f.readline().strip()
+                if first_line:
+                    try:
+                        parse_ssh_public_key(first_line)
+                    except (TypeError, ValueError):
+                        logger.warning(
+                            "Deploy public key file %s is not a valid SSH public key after concurrent creation; ignoring.",
+                            pub_path,
+                        )
+                    else:
+                        _deploy_public_key_cache = first_line
+                        return _deploy_public_key_cache
+            except FileNotFoundError:
+                # Public key file was not found after concurrent creation attempt;
+                # fall back to deriving it from the private key file below.
+                logger.debug(
+                    "Deploy public key file %s not found after concurrent creation attempt.",
+                    pub_path,
+                )
+            except OSError as exc:
+                logger.warning(
+                    "Could not read deploy public key at %s after concurrent creation: %s",
+                    pub_path, exc,
+                )
+
+            derived = _derive_public_key_from_private_file(priv_path, pub_path)
+            if derived is not None:
+                _deploy_public_key_cache = derived
+                return _deploy_public_key_cache
+
+            logger.warning(
+                "Could not recover deploy public key after concurrent creation at %s; "
+                "returning empty string.",
+                priv_path,
+            )
+            _deploy_public_key_cache = ""
+            return ""


In the FileExistsError recovery path, a transient read/derive failure (e.g., another process is still writing the private/public key) ends up caching _deploy_public_key_cache = "", which prevents future calls in this worker from retrying once the concurrent write completes. Consider adding a short retry/backoff before caching the empty string, or leaving the cache as None for this specific branch so a later call can recover.

Copilot · 2026-04-08T17:46:54Z

+    except HTTPException as exc:
+        if request.headers.get("HX-Request"):
+            return HTMLResponse(
+                f'<span class="text-error">{html.escape(exc.detail)}</span>',
+                status_code=200,
+                headers={"HX-Retarget": "#upload-error-msg", "HX-Reswap": "innerHTML"},
+            )


The HTMX error path updates #upload-error-msg via HX-Retarget, but the normal success response for the same request updates only #key-list, so a previously shown error message will remain visible even after a later successful upload. Consider ensuring the success response also clears #upload-error-msg (e.g., include an OOB swap that empties it, or trigger a client-side reset on successful swap).

pacnpal · 2026-04-08T17:47:51Z

@copilot apply changes based on the comments in this thread

…ileExistsError cache fix Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/eddd38db-4de3-4e4a-ada6-3f30675736fe Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-08T17:54:51Z

@copilot apply changes based on the comments in this thread

Applied all three items from review #4077092787 in fd4264d:

Inactive user upload form hidden (user_detail.html) — wrapped the upload form in {% if target_user.is_active %}. When inactive, the form is replaced with "Key management is disabled for inactive users." — both the Generate Key button and the upload form are now suppressed together.
#upload-error-msg cleared on success (key_list.html) — added <p id="upload-error-msg" hx-swap-oob="true"></p> at the top of the key list partial. HTMX strips the OOB element from the swap, applies it to clear the error container, and inserts the key list as normal. This fires on both upload and revoke success.
FileExistsError transient failure no longer cached (core.py) — removed _deploy_public_key_cache = "" from the concurrent-creation derive-failure path. The cache stays None so a subsequent call can retry once the concurrent write completes. The durable PermissionError/OSError write-failure branch still sets the cache to "".

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-08T18:04:26Z

@@ -1,3 +1,4 @@
+<p id="upload-error-msg" hx-swap-oob="true"></p>


The OOB swap used to clear #upload-error-msg replaces the entire <p> element. Since the user_detail.html placeholder defines aria-live, role, and class, this swap will clobber those accessibility/styling attributes. Prefer an OOB swap that only clears the element’s contents (e.g., innerHTML) or ensure the swapped element includes the same attributes as the original placeholder.

Suggested change

<p id="upload-error-msg" hx-swap-oob="true"></p>

<p id="upload-error-msg" hx-swap-oob="innerHTML"></p>

Copilot · 2026-04-08T18:04:27Z

+            return HTMLResponse(
+                '<p class="text-error" role="alert">This key is already registered</p>',
+                status_code=200,
+                headers={
+                    "HX-Retarget": "#generate-key-error",
+                    "HX-Reswap": "innerHTML",
+                },
+            )


Same issue as above: this duplicate-fingerprint HTMX branch returns a <p> but swaps it into #generate-key-error via innerHTML, causing nested paragraphs. Use inline markup/text for the swap target instead.

pacnpal · 2026-04-08T18:05:33Z

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/2438665f-6f4f-4eba-aeba-e190de4c0736 Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-08T18:09:42Z

@copilot apply changes based on the comments in this thread

Applied both items in bb2bc86:

OOB swap innerHTML (key_list.html) — changed hx-swap-oob="true" to hx-swap-oob="innerHTML" so only the content of #upload-error-msg is cleared on success, preserving the aria-live, role, and class attributes on the element.
No nested <p> in generate_key() errors (users.py) — both HTMX error paths that retarget #generate-key-error (the early-validation except HTTPException branch and the duplicate-fingerprint branch) now return plain text instead of a <p> element. Since #generate-key-error is already a <p class="text-error" role="alert">, swapping innerHTML with bare text produces valid HTML.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-08T18:25:46Z

+{% extends "base.html" %}
+{% block title %}Generated SSH Key - {{ instance_name() }}{% endblock %}
+{% block content %}
+<h2>Generated SSH Key</h2>
+<div class="flash flash-error" role="alert">
+  <strong>⚠ Save this private key now.</strong> It will not be shown again once you leave this page.
+</div>
+<p>
+  <strong>Label:</strong> {{ label }} &mdash;
+  <strong>Type:</strong> <code>ssh-ed25519</code> &mdash;
+  <strong>Fingerprint:</strong> <code>{{ fingerprint }}</code>
+</p>
+<div class="ssh-config-actions">
+  <button type="button" class="outline btn-sm" data-copy-trigger="generated-private-key">Copy</button>
+  <button type="button" class="outline btn-sm" data-download-key="id_ed25519" data-key-source="generated-private-key">Download</button>
+</div>
+<pre class="ssh-config" data-copy-target="generated-private-key">{{ private_key }}</pre>
+<p class="text-small text-muted">
+  Save this file as <code>~/.ssh/id_ed25519</code> (or a name of your choice) and set its permissions:
+  <code>chmod 600 ~/.ssh/id_ed25519</code>.
+  Update your SSH config's <code>IdentityFile</code> to point to the file you saved.
+</p>
+<a href="/users/{{ target_user_id }}" role="button" class="secondary">Back to user profile</a>
+{% endblock %}


This template doesn’t appear to be referenced by any route or include (the /users/{id}/keys/generate handler currently rejects non-HTMX requests instead of rendering a full page). If a full-page fallback is intended, wire this template into the non-HTMX path; otherwise consider removing it to avoid dead/unused templates that can drift out of sync with the dialog version.

Suggested change

{% extends "base.html" %}

{% block title %}Generated SSH Key - {{ instance_name() }}{% endblock %}

{% block content %}

<h2>Generated SSH Key</h2>

<div class="flash flash-error" role="alert">

<strong>⚠ Save this private key now.</strong> It will not be shown again once you leave this page.

</div>

<p>

<strong>Label:</strong> {{ label }} —

<strong>Type:</strong> <code>ssh-ed25519</code> —

<strong>Fingerprint:</strong> <code>{{ fingerprint }}</code>

</p>

<div class="ssh-config-actions">

<button type="button" class="outline btn-sm" data-copy-trigger="generated-private-key">Copy</button>

<button type="button" class="outline btn-sm" data-download-key="id_ed25519" data-key-source="generated-private-key">Download</button>

</div>

<pre class="ssh-config" data-copy-target="generated-private-key">{{ private_key }}</pre>

<p class="text-small text-muted">

Save this file as <code>~/.ssh/id_ed25519</code> (or a name of your choice) and set its permissions:

<code>chmod 600 ~/.ssh/id_ed25519</code>.

Update your SSH config's <code>IdentityFile</code> to point to the file you saved.

</p>

<a href="/users/{{ target_user_id }}" role="button" class="secondary">Back to user profile</a>

{% endblock %}

pacnpal · 2026-04-08T19:51:39Z

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/32cc038c-9969-4843-8320-3770732a3c23 Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot · 2026-04-08T19:53:45Z

@copilot apply changes based on the comments in this thread

Done in 916c570 — removed api/templates/generated_key_page.html. Confirmed it had no references in routes, includes, or tests.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

feat: add SSH key generation for callis<->client and callis<->host pa…

c5ef132

…iring Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/4a470c3f-5186-4816-849d-aa0ae0179751 Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

Copilot AI assigned Copilot and pacnpal Apr 7, 2026

Copilot created this pull request from a session on behalf of pacnpal April 7, 2026 15:10 View session

pacnpal marked this pull request as ready for review April 7, 2026 15:10

Copilot AI review requested due to automatic review settings April 7, 2026 15:10

Copilot started reviewing on behalf of pacnpal April 7, 2026 15:11 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Copilot started work on behalf of pacnpal April 7, 2026 18:54 View session

fix: address code review feedback on key generation

cc846e7

Agent-Logs-Url: https://github.com/pacnpal/callis/sessions/35cc0b82-8cb9-4495-9082-69daac104a0d Co-authored-by: pacnpal <183241239+pacnpal@users.noreply.github.com>

github-advanced-security AI found potential problems Apr 7, 2026

View reviewed changes

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Copilot finished work on behalf of pacnpal April 7, 2026 19:01

Copilot AI requested a review from pacnpal April 7, 2026 19:01

Potential fix for pull request finding 'CodeQL / Empty except'

030609d

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 7, 2026 19:38

pacnpal and others added 2 commits April 7, 2026 15:38

Potential fix for pull request finding 'CodeQL / Empty except'

029a8f9

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Potential fix for pull request finding 'CodeQL / Unused global variable'

8c48908

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot started reviewing on behalf of pacnpal April 7, 2026 19:39 View session

pacnpal and others added 2 commits April 7, 2026 15:39

Potential fix for pull request finding 'CodeQL / Unused global variable'

6fdf7bb

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Potential fix for pull request finding 'CodeQL / Unused global variable'

27a4262

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

github-advanced-security AI found potential problems Apr 7, 2026

View reviewed changes

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Comment thread api/core.py Fixed

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Potential fix for pull request finding 'CodeQL / Unreachable code'

fcbada4

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 7, 2026 19:42

Copilot started work on behalf of pacnpal April 7, 2026 19:43 View session

Copilot started reviewing on behalf of pacnpal April 7, 2026 19:43 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes