Skip to content

Keeping the old engine when an update succeeds but turns out bad - and a question about where the web app should call it #670

Closed

Description

@demetree

Title: Keeping the old engine when an update succeeds but turns out bad — and a question about where the web app should call it

Two things, the second optional. I have a prototype of the first in #645 if you want it.

1. A successful engine update has no way back

setup.py is careful about a failed update: update_installed_engine() keeps the installed engine
and starts that instead when the download or the arch check does not work out. That is the right
behaviour, and I did not want to touch it.

The gap is the other direction. In get_prebuilt(), the replacement is:

for p in tmp.iterdir():
    dst = eng / p.name
    if dst.exists():
        shutil.rmtree(dst) if dst.is_dir() else dst.unlink()
    p.replace(dst)

The installed file is deleted and then replaced. If the new engine then turns out to be bad — a crash
only on your card, a regression you hit an hour later, a config that behaves differently — there is no
copy of the old one. The only recovery is re-downloading a release zip by hand.

Two smaller things in the same area:

  • That loop is not all-or-nothing. If it dies partway through, the engine directory holds a mix of old
    and new files.
  • update_install prints Strata is updated (engine 0.1.38) to the console and nothing else. UPDATE.bat
    closes, so afterwards there is nothing to ask — no version before/after, no list of what changed.

I measured what a copy would cost, on the current release (v0.1.38, strata-windows-x64.zip,
124,279,645 bytes as published): strata.exe is 113,479,168 bytes and strata-vision.exe is
108,195,328 uncompressed, so one generation kept on disk is 221,674,697 bytes — 211 MiB. Worth saying
plainly, because it is the whole argument against keeping more than one.

Would you take a backup? Concretely: copy engine/ to engine/.previous (or a timestamped sibling)
before the replace loop, keep one generation, and say on a start which version is installed and which is
kept. That is small and self-contained. My prototype does this, if you would rather look at working code
than a description — but it is a much smaller change than what is in #645.

2. Should the web app be able to update the engine at all?

Not asking for it. Asking whether it is wanted, because I could not work out where you would want it to
live, and I would rather not guess.

I had assumed updating was manual — download the release zip, replace the files. It is not:
update_installed_engine() runs on every START-HERE.bat (setup.py:3042, 3058), and UPDATE.bat /
update.sh (#475) does it deliberately without starting the model. And get_prebuilt() already does
what I thought was the clever part — it unpacks to engine/_unpack and checks the release's own
archs against your GPU before installing, falling back to compiling. So "update the engine" is solved,
twice, and better than my first attempt at it.

What genuinely is missing is doing it from the page the user is already on, and seeing it happen: the
engine is a 124 MB download, the model must be stopped first (the server knows whether it is loaded, and
whether a request is in flight — unload() already refuses with "busy"), and a console window that
closes is not much of a progress display.

Three answers I can see:

  • (a) Reuse it. The web route shells out to setup.py --update and relays its output. Nothing to
    keep in sync, no second arch rule. Cost: a console process from the server, and progress means parsing
    text that is not a contract.
  • (b) Keep it in serve/, as Update the engine from the web app, cautiously #645 does. Real progress, no shelling out, and the server knows the
    state that matters. Cost: a second implementation of download-verify-replace, and a second copy of the
    arch rule.
  • (c) Neither — the web app shows the installed version and whether a newer release exists, and
    leaves updating itself to UPDATE.bat. A few dozen lines, no duplication, but no progress and no backup.

If you have a preference I will reshape #645 to match rather than defend the shape it happens to be in.
If the answer is (c), most of #645 should be deleted, which I am fine with.

3. Two small ones, separate from the above

  • BUILD.json with a BOM. json.loads(path.read_text()) at serve/server.py:322 raises on the
    three BOM bytes, so an engine whose BUILD.json was saved by a Windows editor, or by PowerShell's
    Set-Content -Encoding utf8, reports no version — which the About tab then shows as "built from
    source". One line: encoding="utf-8-sig", as setup.py already does for some of its configs. I have
    deliberately left it out of Update the engine from the web app, cautiously #645 rather than smuggle it in; happy to send it as its own small PR.
  • No checksums on the release. Neither the assets nor BUILD.json carries one, so there is no way to
    confirm a downloaded engine is the published one — only that it is not truncated and that it starts.
    A sha256 alongside each asset would close that. I did not include the release-side change, because
    publishing a hash nothing checks is worse than none; say the word and I will write it.

Activity

  1. Niko1221 commented on Oct 6, 2026

    @Niko1221
    Owner

    Fixed in 0.1.40: an engine update keeps the engine it replaces in engine/.previous (one generation, ~210 MiB; the swap is all or nothing), and python setup.py --rollback-engine puts it back. Updating from the web app is not planned.

  2. demetree commented on Oct 6, 2026

    @demetree
    ContributorAuthor

    Part 1 is done — and it changes what is left in #1162

    setup.py --rollback-engine and engine/.previous have landed (setup.py's #670), so the "a successful
    update has no way back" half of this is answered. Thank you — that was the more useful of the two things
    I asked about, and having read it I would not have built it the way you did: keeping engine/.previous
    means get_prebuilt()'s own swap already carries it, and my in-app backup was solving a problem the
    project had already solved, in a second place.

    So the remaining question from part 2 is narrower than it was:

    #1162 now adds a SHA-256 check on the engine download, and it is the thing that makes a web-side update
    safe at all.
    Without it, an update reachable from a browser is a remote code execution path — your words
    on #645, and correct: the archive runs with the user's rights, so anyone who can alter the download (a
    compromised mirror or asset, a proxy, a hostile network) gets code execution. Checking the byte count,
    which is all get_prebuilt() had, does not help against that; a substituted file of the same length
    passes.

    GitHub publishes the hash, so this needs no release-process change: the releases API returns digest
    (sha256:<hex>) for every asset on every release — I checked v0.1.34 through v0.1.40.1. The value
    arrives on a different connection from the file, which is what makes it worth having.

    The question I would still like answered: should get_prebuilt() do the same check? It is the path
    every normal install takes — START-HERE.bat on a start, UPDATE.bat deliberately — and today it installs
    an unverified engine. It is the same one-line lookup of digest against the same API, so it is cheap to
    add, but it means a GitHub API call on the setup path where there is currently only a plain
    releases/latest/download/... URL. I have not done it, because it changes install behaviour for everyone
    and that is your call rather than mine.

    If you would rather the whole thing live in one place, the other option stands: the web route shells out
    to setup.py --update and relays its output. Much smaller diff, nothing to keep in sync, no second arch
    rule — at the cost of a console process from the server, and progress by parsing text that is not a
    contract. I am happy to reshape #1162 either way; I would rather match your architecture than defend mine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions