git: use unique temporary SSH key paths

[HoroTW]

Summary

This changes SSH credential handling for Git operations to use a unique temporary key file per operation instead of reusing /tmp/.ssh-key-<credentialId>.

Problem

Concurrent Git operations that use the same SSH credential currently share the same temporary key path. When one operation finishes, it removes that shared key file while another operation may still need it. This can make parallel syncs fail with SSH key lookup errors, especially during blobless clone/lazy fetch paths.

Fix

• Add a per-operation suffix using the process id and randomUUID().
• Store the generated SSH key path in the Git environment.
• Clean up the exact generated path for the operation that created it.
• Keep the old deterministic path only as a fallback for cleanup callers without an env.

Verification

• git diff --check HEAD~1..HEAD
• Deployed an equivalent runtime patch on a v1.0.31 Dockhand container and smoke-tested concurrent/shared-credential Git stack syncs successfully.

No database, API, or config changes.

[CLAassistant]

All committers have signed the CLA.

[HoroTW]

This is the Telegram notification I got every 10m (auto sync interval) before the fix, as far as I see it, it only happens in mono repos with ~5-10 Stacks in one Repo.

Not sure if that is not really a supported way, but from the docs I thought it was and that it would be nice to have one repo for all of my stacks (instead of individual repos) - maybe there are reasons to not do something like that?

Cheers
~Horo

[radoeka]

This is the Telegram notification I got every 10m (auto sync interval) before the fix, as far as I see it, it only happens in mono repos with ~5-10 Stacks in one Repo.

I do it the exact same way. That means I now have 27 stacks/compose.yaml files in my repository.
It would indeed be nice to know if this is supported (or not).

[HoroTW]

@radoeka you encountered the same issue as me, right?

[radoeka]

I did not encounter problems (yet).
I just wanted to tell that more people have a setup with many stacks in 1 repository.

[HoroTW]

Hmm, okay. It's surprising to me that this straightforward use case has such an issue, so maybe it's because I'm using Forgejo or there's another reason.

[radoeka]

I think that your PR is a good one, perhaps also more secure(?). I don't update automatically, or just one by one.

I want to move to forgejo (already running) and move to 1 stack per repository at the same time, as that seems to be the expected approach.

[jotka]

Thanks for digging into this.

The unique SSH key path fix is a good fix — clean, surgical, and clearly addresses the shared-/tmp/.ssh-key-{id} race you described. Happy to land that part.

We have concerns about the locking layer though, which isn't mentioned in the PR summary:

• Removing the syncStatus === 'syncing' short-circuits changes syncGitStack from "skip if busy" to "queue and wait." For HTTP-triggered paths (webhooks, manual deploy) that previously returned a fast 4xx, callers now block — a slow remote plus a 10-min auto-sync interval can hold a stack lock most of the window with each tick queuing instead of skipping. There's no upper bound on queue depth either.
• It introduces a second concurrency model that competes with the existing DB-based one. We rely on the syncStatus field surviving restarts, and hooks.server.ts has explicit startup recovery that resets stuck 'syncing' rows. With an in-process Map, none of that carries over, and it's not clear which is authoritative.
• In-process locks don't span workers or survive restarts. Single-process today, but it's a constraint worth being explicit about.
• The runSync body in syncGitStackInternal, deployGitStack, and deployGitStackWithProgress isn't re-indented after the wrapping — makes the diff harder to review.

Could you confirm whether the unique-path fix alone stopped the Telegram alerts? Your description traces the failure entirely to the shared key being removed mid-fetch, which # 1 fully addresses — so we suspect # 2 may not be needed to fix your reported issue.

Could you split this into two PRs? We'd merge the unique-path fix right away; the locking change can have its own discussion where we can work through the queue semantics and how they interact with syncStatus.

[HoroTW]

Hey hey,
Correct, the first part was the issue, I think I have a different issue with my server instance, but that is probably not related to dockerhand. I will change this PR back to # 1

For # 2 I'll approach it from a different angle first to ensure I don't have general issues (e.g. I noticed my network connectivity might be flakey)