Maybe add a comment to state that bits & 0x07 check is to prevent malleability ?

It's a bit more than that: the SHA256 class is simply incapable of processing sub-byte data as it stands. But I can write a comment to that effect.

I mean you could just just do bits &= ~0x07 and not do the check, but this would introduce non-determinism, and the comment could tell the reader why it is not a good idea

I had hoped it goes without saying that one shouldn't mangle an invalidly serialized data blob into a valid one.

validity of serialized data depends on the definition of the validity, but it is not explicitly specified (the serialization code in Save() is only one possible implementation of implicit specification). But, on the other hand, you can view the (bits & 0x07) != 0 as the statement of the de-facto specification "low-order bits should be zero", and in this light, no extra comment seems necessary.

Checking for SHA256_MAX - MAX_SCRIPT_ELEMENT_SIZE in Load() would allow to get rid of three ifs (if !ctx.SafeWrite(...) -> ctx.Write(...)), because Write() will be always safe in regard to this check: the size of data on the stack cannot be bigger than MAX_SCRIPT_ELEMENT_SIZE. Why you think that doing this in Write() is better ? Note that in OP_SHA256_INITIALIZE it is even impossible to fail this check, because the CTX is new and can only increase up to MAX_SCRIPT_ELEMENT_SIZE. I think an if check is due in Load() and here this condition can just be checked with assert to be safe.

Is it because you want to allow for UPDATE with user-supplied midstate to be able to go up to SHA256_MAX exactly ?

You cold make Load() take the length and then check SHA_MAX - bytes < len inside Load().
Although this makes Load/Save api less generic. But if you want it to be more generic so it can be used in other places, you probably want that check inside Load() anyway, because in general you cannot prevent anyone to use just Write() instead of SafeWrite() after Load()

I suppose I could write Load to return a int64_t that returns the number of bytes used so far (or alternatively the number of remaining bytes available to be processed), and return -1 for failure.

Is this really all worthwhile just to avoid one if statement in SHA256INITALIZE?

I suppose we also get rid of SafeWrite which is kinda nice to get rid of.

all three if statements that are in place for SafeWrite now are not needed if Write is always safe, if the check SHA_MAX - bytes < len is done inside Load() - at least in regard to manipulating the state from the script. The Load() can just fail if the subsequent Write() will be invalid. But my opinion is not strong on this - this is mostly bikeshedding. But I would like to add that check inside Load() would follow 'fail early' principle, which I favour.

Having Load take in an argument of the number of bytes I intend to write later is such an unusual API, that I am very hesitant to implement it that way.

Just rename Load to Prepare then :-)

As I said, my opinion on this is not strong. Speaking of returning the number of bytes still available to process from Load and then checking it might be more messy than SafeWrite approach, because you will need to have that check in two places (in UPDATE and FINALIZE)