fix(egress): the unattributed message must name the token-mismatch cause too
`/resolve` fail-closes on a missing/ambiguous registry row *and* on a request whose identity token doesn't match. The message named only the first, so a bottle that was registered correctly but sent no token read as "not registered" and sent the reader looking for a deregistered bottle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -418,10 +418,11 @@ class PolicyResolverLike(typing.Protocol):
|
|||||||
# to begin with — the failure mode that made a bricked registration read like
|
# to begin with — the failure mode that made a bricked registration read like
|
||||||
# a misconfigured allowlist.
|
# a misconfigured allowlist.
|
||||||
DENY_UNATTRIBUTED = (
|
DENY_UNATTRIBUTED = (
|
||||||
"egress: this bottle is not registered with the orchestrator, so it has "
|
"egress: this request was not attributed to any bottle, so no egress "
|
||||||
"no egress policy at all and every host is denied. The bottle's registry "
|
"policy applies and every host is denied. Either the bottle's registry "
|
||||||
"row is missing or ambiguous — it was torn down, or another bottle claimed "
|
"row is missing/ambiguous (torn down, or another bottle claimed its "
|
||||||
"its source IP. Relaunch the bottle; this is not an allowlist problem."
|
"source IP), or the request carried no matching identity token — check "
|
||||||
|
"that the caller's proxy URL includes it. This is not an allowlist problem."
|
||||||
)
|
)
|
||||||
DENY_UNPARSEABLE = (
|
DENY_UNPARSEABLE = (
|
||||||
"egress: this bottle's egress policy could not be parsed, so it is being "
|
"egress: this bottle's egress policy could not be parsed, so it is being "
|
||||||
|
|||||||
@@ -130,13 +130,17 @@ class TestDenyReasonNamesTheRealFault(unittest.TestCase):
|
|||||||
cfg = resolve_client_config(resolver, "10.243.0.1") # type: ignore[arg-type]
|
cfg = resolve_client_config(resolver, "10.243.0.1") # type: ignore[arg-type]
|
||||||
return decide(cfg.routes, host, "/v1/x", {}, deny_reason=cfg.deny_reason).reason
|
return decide(cfg.routes, host, "/v1/x", {}, deny_reason=cfg.deny_reason).reason
|
||||||
|
|
||||||
def test_unattributed_says_unregistered_not_allowlist(self) -> None:
|
def test_unattributed_says_unattributed_not_allowlist(self) -> None:
|
||||||
reason = self._reason(_FakeResolver(result=None))
|
reason = self._reason(_FakeResolver(result=None))
|
||||||
self.assertEqual(DENY_UNATTRIBUTED, reason)
|
self.assertEqual(DENY_UNATTRIBUTED, reason)
|
||||||
# The misleading claim is the one that must be gone: the host was
|
# The misleading claim is the one that must be gone: the host was
|
||||||
# never "not in the allowlist" — there was no allowlist at all.
|
# never "not in the allowlist" — there was no allowlist at all.
|
||||||
self.assertNotIn("is not in the bottle's egress.routes allowlist", reason)
|
self.assertNotIn("is not in the bottle's egress.routes allowlist", reason)
|
||||||
self.assertIn("not registered", reason)
|
# Both causes must be named. `/resolve` fail-closes on a missing row
|
||||||
|
# *and* on a token mismatch, and the message pointing only at the row
|
||||||
|
# sent us hunting for a deregistered bottle that was registered fine.
|
||||||
|
self.assertIn("registry row", reason)
|
||||||
|
self.assertIn("identity token", reason)
|
||||||
|
|
||||||
def test_resolver_error_says_orchestrator_unreachable(self) -> None:
|
def test_resolver_error_says_orchestrator_unreachable(self) -> None:
|
||||||
self.assertEqual(DENY_RESOLVER_ERROR, self._reason(_FakeResolver(raises=True)))
|
self.assertEqual(DENY_RESOLVER_ERROR, self._reason(_FakeResolver(raises=True)))
|
||||||
|
|||||||
Reference in New Issue
Block a user