Compare commits

..
Author SHA1 Message Date
Hermes AgentandClaude Opus 4.8 06d6828d73 fix(mosaic): de-flake mutator-class lease gate TTL-expiry test
ci/woodpecker/pr/ci Pipeline was successful
The "observer revocation and monotonic TTL expiry deny the next
mutator" acceptance test reused a 1-second-TTL lease both to prove
authorization works immediately after promotion and, later, to
demonstrate real monotonic-clock expiry. Under a contended push-CI
host (concurrent build-gateway/appservice/web matrix), scheduling
delay alone between promote() and the immediate "still valid"
authorize() check could consume the entire 1-second margin, flipping
that setup assertion to a spurious deny (Woodpecker push CI#1945:
`expected { ok: false } to match { ok: true, decision: 'allow' }`).

Reproduced deterministically by inserting a 995ms delay between
promote() and the check, which reliably reproduced the exact CI#1945
failure signature.

Fix is test-determinism only: the initial "lease authorizes" check
now uses a normal-TTL lease with no real-time race, and a dedicated,
isolated short-TTL lease drives only the deliberate wait-then-expire
assertion. No production lease-broker/mutator-gate code changed, and
the deny-the-next-mutator security assertions (LEASE_EXPIRED after
TTL wait, MUTATOR_UNVERIFIED after revoke) are unchanged and
unweakened.

Closes #860

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BMpAGx3YQZT64dov5xCNJL
2026-07-20 05:16:40 -05:00
4 changed files with 28 additions and 41 deletions
@@ -91,19 +91,13 @@ remote = urlparse(f"//{remote_host}")
if configured.scheme not in {"http", "https"} or configured.hostname != remote.hostname: if configured.scheme not in {"http", "https"} or configured.hostname != remote.hostname:
raise SystemExit(1) raise SystemExit(1)
# Normalize by scheme: an implicit (portless) HTTP(S) URL and its explicit configured_port = configured.port
# default-port form (":80" for http, ":443" for https) name the same remote_port = remote.port
# provider endpoint. Apply that equivalence symmetrically -- whichever side if remote_port is None:
# omits the port is treated as carrying the scheme's default port -- so
# "configured implicit vs. remote explicit" and "configured explicit vs.
# remote implicit" both match. (The remote side here is always an HTTP(S)
# authority; an SSH remote's transport port is stripped by get_remote_host
# before reaching this comparison, since it identifies an unrelated
# service on the same host, not the HTTP(S) provider port.)
default_port = 80 if configured.scheme == "http" else 443 default_port = 80 if configured.scheme == "http" else 443
normalized_configured = configured.port if configured.port is not None else default_port if configured_port not in {None, default_port}:
normalized_remote = remote.port if remote.port is not None else default_port raise SystemExit(1)
if normalized_configured != normalized_remote: elif configured_port != remote_port:
raise SystemExit(1) raise SystemExit(1)
raise SystemExit(0) raise SystemExit(0)
PY PY
@@ -438,11 +432,7 @@ get_remote_host() {
fi fi
if [[ "$remote_url" =~ ^ssh://([^/]+)/ ]]; then if [[ "$remote_url" =~ ^ssh://([^/]+)/ ]]; then
local host="${BASH_REMATCH[1]}" local host="${BASH_REMATCH[1]}"
host="${host##*@}" echo "${host##*@}"
# Strip an SSH transport port (e.g. "git.example:2222"): it names the
# SSH daemon port, not the HTTP(S) provider API port, and must not
# feed gitea_url_matches_host's port comparison (#850).
echo "${host%%:*}"
return 0 return 0
fi fi
if [[ "$remote_url" =~ ^git@([^:]+): ]]; then if [[ "$remote_url" =~ ^git@([^:]+): ]]; then
@@ -73,7 +73,7 @@ case "${PR_REVIEW_TEST_MODE:-}" in
request-changes) request-changes)
[[ "$*" == "pr reject 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 91 [[ "$*" == "pr reject 123 --repo mosaicstack/stack --login mosaicstack" ]] || exit 91
;; ;;
legacy-fallback|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|write-transport-failure|write-http-failure|readback-failure) legacy-fallback|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|write-transport-failure|write-http-failure|readback-failure)
if [[ "$*" == pr\ comment* ]]; then if [[ "$*" == pr\ comment* ]]; then
# tea v0.11.1 treats the nonexistent subcommand as `tea pr list` and exits 0. # tea v0.11.1 treats the nonexistent subcommand as `tea pr list` and exits 0.
printf '%s\n' 'INDEX TITLE STATE' printf '%s\n' 'INDEX TITLE STATE'
@@ -144,7 +144,7 @@ case "${PR_REVIEW_TEST_MODE:-}" in
write-http-failure) write-http-failure)
write_response 500 '{"message":"simulated rejection"}' write_response 500 '{"message":"simulated rejection"}'
;; ;;
approve|request-changes|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|ssh-transport-port-success|explicit-default-port-success|readback-failure) approve|request-changes|comment-success|http-success|prefix-success|subpath-success|port-success|scp-ssh-success|url-ssh-success|readback-failure)
if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then if [[ "$method" == "POST" && "$url" == "$PR_REVIEW_EXPECTED_API_BASE/issues/123/comments" ]]; then
PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY' PR_REVIEW_PAYLOAD="$payload" python3 - <<'PY'
import json import json
@@ -291,23 +291,6 @@ grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$'
run_review url-ssh-success comment durable-body https://git.example ssh://[email protected]/owner/repo.git owner/repo run_review url-ssh-success comment durable-body https://git.example ssh://[email protected]/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG" grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
# #850 (follow-up to #812): an SSH remote's transport port (e.g. `ssh://
# git@host:2222/...`) must NOT be compared against the configured HTTP(S) API
# URL's port -- they identify unrelated properties (SSH daemon port vs. HTTP(S)
# provider port) of the same Gitea host. Before the fix, host-match required
# the configured URL to carry the identical port, so this failed closed even
# though both remote and configured URL name the same host.
run_review ssh-transport-port-success comment durable-body https://git.example ssh://[email protected]:2222/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
# #850 (follow-up to #812): an explicit default HTTP(S) port on the remote
# (`https://host:443/...`) must be treated as equal to an implicit
# (portless) configured URL on BOTH sides -- the pre-fix comparison only
# normalized the default port when the REMOTE side was portless, so the
# inverse (explicit remote, implicit configured) form failed closed.
run_review explicit-default-port-success comment durable-body https://git.example https://git.example:443/owner/repo.git owner/repo
grep -q '^POST https://git.example/api/v1/repos/owner/repo/issues/123/comments$' "$CURL_LOG"
if run_review write-transport-failure comment durable-body; then if run_review write-transport-failure comment durable-body; then
echo "Expected provider transport failure to return nonzero" >&2 echo "Expected provider transport failure to return nonzero" >&2
exit 1 exit 1
+1 -1
View File
@@ -25,7 +25,7 @@
"lint": "eslint src", "lint": "eslint src",
"typecheck": "tsc --noEmit", "typecheck": "tsc --noEmit",
"test": "vitest run --passWithNoTests && pnpm run test:framework-shell", "test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
"test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh" "test:framework-shell": "python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh"
}, },
"dependencies": { "dependencies": {
"@mosaicstack/brain": "workspace:*", "@mosaicstack/brain": "workspace:*",
@@ -661,13 +661,27 @@ describe('whole mutator-class lease gate', () => {
test('observer revocation and monotonic TTL expiry deny the next mutator', async () => { test('observer revocation and monotonic TTL expiry deny the next mutator', async () => {
const { socket } = await startBroker(); const { socket } = await startBroker();
const sessionId = await register(socket); const sessionId = await register(socket);
const pending = await beginVerification(socket, sessionId, 'claude', 1, 1);
await promote(socket, sessionId, pending.receipt_challenge!);
// Establish the lease with a normal (non-racing) TTL first and prove it
// authorizes. This "still valid" check is setup, not a TTL-expiry
// assertion, so it must not share a lease with a 1-second TTL: on a
// contended push-CI host, scheduling delay alone between promote() and
// this authorize() call can consume that entire 1-second margin and
// spuriously deny it (CI#1945). Using a generous TTL here removes that
// real-time race without touching lease-gate security semantics.
const pending = await beginVerification(socket, sessionId, 'claude');
await promote(socket, sessionId, pending.receipt_challenge!);
expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({ expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({
ok: true, ok: true,
decision: 'allow', decision: 'allow',
}); });
// A dedicated, isolated short-TTL lease drives the deliberate monotonic
// expiry demonstration below. It is never used for anything but the
// wait-then-expire assertion, so there is no setup work racing its
// 1-second window.
const shortLived = await beginVerification(socket, sessionId, 'claude', 1, 1, 2);
await promote(socket, sessionId, shortLived.receipt_challenge!);
await new Promise((resolve) => setTimeout(resolve, 1_100)); await new Promise((resolve) => setTimeout(resolve, 1_100));
expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({ expect(await authorize(socket, sessionId, 'claude', 'Bash')).toMatchObject({
ok: false, ok: false,
@@ -675,7 +689,7 @@ describe('whole mutator-class lease gate', () => {
decision: 'deny', decision: 'deny',
}); });
const refreshed = await beginVerification(socket, sessionId, 'claude', 1, 300, 2); const refreshed = await beginVerification(socket, sessionId, 'claude', 1, 300, 3);
await promote(socket, sessionId, refreshed.receipt_challenge!); await promote(socket, sessionId, refreshed.receipt_challenge!);
expect( expect(
await request(socket, { await request(socket, {