Skip to content

fix(master): reject revoked nodes after long-poll wake - #440

Open
Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/node-revoke-race
Open

Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/node-revoke-race

Conversation

@Floating-Y

Copy link
Copy Markdown
Collaborator

A node revoked while its idle poll was held could resume after certificate validation, mark itself connected again, and proceed to command dispatch.

Make the health UPDATE conditional on revoked=0 and have shared diff return 403 when no node is updated. The check runs at the shared state write, so a resumed poll cannot reconnect a revoked node or reach take_execs. The listening-node driver already propagates the same error.

Add identity regression coverage using an explicit oneshot signal after the request enters its hold, without fixed sleeps. Assert that the held and subsequent polls return 403, the node stays offline, and ordinary automatic placement returns 503 when it is the only candidate. Keep a healthy-node control that wakes and receives a Create command. Extend state coverage for rejected revoked reports and health updates after certificate registration.

Validation:

  • Before the fix on ef7f84d, the regression failed: the held poll returned 200 instead of 403.
  • Rebased onto upstream dev d27ab7c; the patch is unchanged.
  • cargo test -p sandstorm-core --test node_identity --test master_api --test listen_mode: 87 passed (13 identity, 68 master API, 6 listening mode).
  • cargo test -p sandstorm-core --lib state::tests: 24 passed.
  • cargo test -p sandstorm-core --lib master::drive: 2 passed.
  • cargo fmt --all --check and diff whitespace checks passed.

Linux tests ran in WSL Ubuntu. The full repository suite was not run. A zero-row health update, including an unregistered node, now returns 403.

Signed-off-by: danwood <118035379+Floating-Y@users.noreply.github.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant