Improve etcd-based RW locks and API contextual awarness of cancelation

[pandatix]

What is the problem/feature ?

Throughout many internal usage at CTFer.io, we often deployed single instances of Chall-Manager.
Given this context, we often relied on a local lock. This is also the default setting, and is not documented else way.

Nonetheless, it is mandatory for production purposes to use etcd at it needs scaling and recoverability in case of a transient failure.
For instance, #1074 shows people are actually using the etcd-based RW locks 👍

Even though there seem to be no major issues with the triple RW lock chain, this mostly hold to nothing as many assumptions were maid. One of them is that a request cannot be canceled, which is not realistic (e.g. a upstream service with a timeout)...

In such cases, we often observed recovery after a while, but was not thanks to automated recovery in Chall-Manager... It has been due to etcd session closing, revoking the locks, and by luck did not break the RW locks' counters.

So, the problem is that etcd-based RW locks do not handle context cancelation, nor does the API.

What contains this PR ?

This PR is quite heavy, as it improves drastically the contextual awarness of the business layer (aka the /api directory) and the etcd RW locks implementation.
Sadly, it contains too much to simply write a meaningfull TL;DR... but it improves etcd-based RW locks A LOT.

Time-attack bugs

I discovered many ways of soft-locking etcd-based RW locks based upon time-attacks context cancelations.
It is difficult to actually exploit in production systems reliably due to networking delays and pseudo-random assumptions on RW locks parallelization timings, so I don't consider exploitation possible, tho we might discuss it more thorously later. An attacker might spam all endpoints with random timeouts until something happen, at best...

Through careful review, I especially discovered 3 bugs:

1. a double challenge RW unlock on file system read failure when deleting a challenge;
2. a race condition on challenge filesystem deletion that can lead to a zombie challenge, require precise timing. The end of challenge delete operation was not protected, so it was possible to start creating a challenge that errors due to remaining files from an ongoing deletion. Low probability as the delete end section would have to be executed way more slowly that the create one, yet still possible;
3. a missing TOTW unlock on file system read failure when querying instances;

The first is easily recoverable and won't cause a lot of trouble (I think, or guess).
The second one is recoverable but can lead to strange transient errors, hindering atomicity of API operations thus predictability.
The last would cause trouble, as it would be possible to soft-lock Chall-Manager, but it is out of upstream services' scope so I don't consider it a vulnerability.

Etcd

For each lock created as part of an API request, there was a set of sessions opened and later closed through response (between 1 -for the TOTW- and m+n+1 with m the number of challenges and n the total number of instances).
While this worked, it spammed etcd with many useless work.
Moreover, in case a session is terminated, the locks where automatically freed thus future unlocks would end up erroring (i.e., already unlocked).
For these reasons, I move the session management to the global etcd service management and hide underlying complexities of renewing it.

When locks where used, the context was used blindly, but might have been canceled by the upstream service (e.g., timeout).
This might have led to un-performed steps in the recovery mecanisms of the current implementation...
With this PR, they now consider the context to be cancellable and deal with every situation it might have been canceled such that it recovers and maintain a transation-like behavior.

For healthchecks, current implementation performs a get on health key in etcd, sometimes multiple times for each usage of the etcd service!
This spams etcd A LOT for no specific reason: we can deal with the assumption that it was recently fine, thus skip checking again. Actually, I arbitrarly estimated "fine" to be 10 seconds.
That way, we reduce the load on etcd and improve performances.

Finally, we sometime observed long-running requests around 40 seconds... But never in production 🤔
After analysis, it might have been due to no keepalive mecanism used on the etcd client, thus once the system is idle for too long (i.e., more than 5 minutes in the default settings) it hits all timeouts and need to recreate every connection...
For this reason, I activate the keepalive mecanism of the etcd client in the etcd service manager, and confirm that after 5 minutes the first request is not a long-running one.
In production, this was not a problem as the janitor issued at least 1 API call every minute, so way below deadlines.

Oh, not to forget that I clarify the current assumptions for etcd-based locks: unqueued/unfair locks, rely on a stable network connection.

Overall, the use of etcd I implemented months ago sucked 🙃

API

In the "business" layer of Chall-Manager, the context cancelation is now a handled case, similarly to the etcd locks.
Nonetheless, it showed room for improvements:

• we might shortcut operations on filesystem or avoid spawning Pulumi processes in case we already know the context has been canceled, saving time and resources;
• some paths containing errors might lead to unstable state for challenges or their instances, i.e., incomplete operations with infrastructure deployed but nothing written on disk.

Some observations

• if you ran Chall-Manager with etcd in production, and ONE request failed (or is canceled, e.g., with a timeout on a busy resource) you might softlock everything 🙃;
• these explanations are quite long, I'll pay Nicolas a drink if he reads this;
• this PR is not thoroughly tested as it is literally impossible (too many logic paths and edge cases to simulate). The only way is using formal proofing of the lock logic, which I keep as a future work (if anyone want to collaborate on this, please reach out!);
• future work might also include improved recoverability on counters, especially since I now realize the etcd RW locks' assumptions are very strong (especially network issues and session rotation).

What are the impacts ?

The etcd-based setup for RW locks do not longer spams error logs when running, and recover way better in case of request cancelation.

Resolves #1074

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-lint (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Jan 12, 2026, 9:28 PM

[coveralls]

Pull Request Test Coverage Report for Build 20935595152

Details

• 124 of 689 (18.0%) changed or added relevant lines in 13 files are covered.
• 21 unchanged lines in 7 files lost coverage.
• Overall coverage decreased (-2.1%) to 50.207%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/services/etcd/manager.go	47	48	97.92%
pkg/lock/local.go	8	19	42.11%
api/v1/challenge/query.go	3	18	16.67%
api/v1/instance/query.go	1	18	5.56%
api/v1/challenge/create.go	2	26	7.69%
api/v1/challenge/retrieve.go	0	26	0.0%
api/v1/challenge/update.go	3	33	9.09%
api/v1/instance/renew.go	3	33	9.09%
api/v1/challenge/delete.go	7	43	16.28%
api/v1/instance/retrieve.go	4	49	8.16%

Files with Coverage Reduction	New Missed Lines	%
api/v1/instance/spin.go	2	47.66%
pkg/services/etcd/manager.go	2	87.04%
sdk/kubernetes/exposed-multipod.go	2	91.69%
global/log.go	3	80.0%
api/v1/challenge/query.go	4	59.87%
pkg/errors/internal.go	4	0.0%
pkg/lock/etcd.go	4	23.3%

Totals
Change from base Build 20881617546:	-2.1%
Covered Lines:	4478
Relevant Lines:	8919

---

💛 - Coveralls

[NicoFgrx]

the excalidraw is not renderer properly