Skip to content

fix(api): potential softlocks on unreleased R challenge locks - #1486

Open
pandatix wants to merge 2 commits into
mainfrom
fix/api-soft-lock
Open

pandatix wants to merge 2 commits into
mainfrom
fix/api-soft-lock

Conversation

@pandatix

@pandatix pandatix commented Oct 8, 2026

Copy link
Copy Markdown
Member

This PR adds missing clock.Runlock calls. Current implementation can lead to softlocks, such that only read operations on challenges are possible in the future. In case of local lock a reboot is sufficient to fix, yet as it is stateful it is nearly impossible in production ; etcd needs Ops deleting the locks manually, which suffers from the previous plus the complexity of etcd itself.

It also fixes the linting errors that appeared a while ago through a silent update in golangci-lint, blocking again the dependabot PRs.

Found by @d3vyce

@pandatix pandatix added bug Something isn't working go Pull requests that update Go code chall-manager Related to chall-manager lock/etcd When Chall-Manager uses etcd as the distributed lock backend. lock/local When Chall-Manager uses a local lock. labels Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedOct 8, 2026, 1:52 PM

@pandatix
pandatix requested a review from NicoFgrx October 8, 2026 14:26
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 37787800131

Warning

No base build found for commit 16d206c on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 37.589%

Details

  • Patch coverage: 35 uncovered changes across 5 files (0 of 35 lines covered, 0.0%).

Uncovered Changes

File Changed Covered %
api/v1/instance/delete.go 18 0 0.0%
api/v1/instance/create.go 11 0 0.0%
api/v1/challenge/query.go 2 0 0.0%
api/v1/challenge/retrieve.go 2 0 0.0%
api/v1/challenge/update.go 2 0 0.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 9689
Covered Lines: 3642
Line Coverage: 37.59%
Coverage Strength: 0.41 hits per line

💛 - Coveralls

@d3vyce

d3vyce commented Oct 8, 2026

Copy link
Copy Markdown

Tested, it fixes the softlock 👌

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

Labels

bug Something isn't working chall-manager Related to chall-manager go Pull requests that update Go code lock/etcd When Chall-Manager uses etcd as the distributed lock backend. lock/local When Chall-Manager uses a local lock.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants