Assert every permitted name reaches the set its class names #356

Merged

branderson merged 3 commits from verify-entries into development 2026-09-15 19:47:09 -07:00

Conversation 1 Commits 3 Files Changed 2

+236 -22

dejima-agent commented 2026-09-14 23:14:31 -07:00

Collaborator

Copy Link

Copy Source

Summary generated by LLM agent

Closes the gap that four review rounds kept falling into, by asserting the property in a running container instead of inferring it from reading.

What was only checked for one destination

Check 4 asserted that a permitted name resolves and its address reaches the address set its class names. It did that for exactly one host: the one in the GitHub metadata URL, which is the destination hardest to get wrong. The operator's own forge, the Loki host, a self-served backend and every approved domain were never subjected to it.

That is where the defects have been. The class an entry was recorded with, the resolver line built from it, and the set the ports are keyed to are three separate statements, and each can be right about a different set. Reading the code cannot tell them apart, which is why every defect of this shape reached a green suite:

• a forge name recorded in the open class, so its answers fed the port-unrestricted set
• a forge name skipped out of every class, so internal_domains could not find it
• a name that could not resolve at all, because the resolver will not forward a dotless query

All three present at boot in the same way: a name that answers, and an address that is not where the ports expect it. One check catches all three, in the real container, before the session starts.

The change

The sweep that already resolves every permitted name now reads each answer back out of its class's set.

It is fatal, unlike a name that returns no address. That distinction is deliberate and kept: an unresolvable name is the world being unavailable, and refusing to start a session over a host that happens to be down would cost more than it prevents. A name that resolves to an address the policy did not permit in the class it was recorded under is the policy disagreeing with itself, and the session it produces reaches a configured destination only by accident, or not at all.

Which set a class feeds now has one definition. The config that fills a set and the check that reads it back asking separately is the same shape as the bug: they could name different sets and the check would pass while the class meant nothing.

One name, one class

Writing the check found that this repository's own allowlist would fail it.

A name can reach both class lists: release-assets.githubusercontent.com is an open-class entry of the component's own and also sits under a forge suffix, so the approved-domains loop adds it to the forge class as well. Any harness or backend domain under a forge suffix would do the same. That produced two entries for one name, two resolver lines naming different sets, and one of them honoured - so nothing could say which class was in force, and the new check would look in the set that lost and stop the launch.

Entries are now deduplicated by name, keeping the first, and forge is recorded first. A name that is forge class by any route keeps the port restriction rather than inheriting the open class from a second mention. That also makes fixed-domains.sh's claim that the forge class wins wherever the two overlap true of the entries file, which it was not.

This is the item recorded as the third finding on #350.

Testing

Full confined gate, all ten suites, green.

The new assertions were each run against their own removal: dropping the membership check lets a stranded name pass, giving the two places their own set name defeats the single-definition test, and removing the deduplication produces two entries for the one name.

The stranded case is covered in both shapes - an address in no set at all, and a forge-class name whose address is in the open class's set, which is the one that actually happened and which a check asking only "is it permitted at all" would miss.

The harness for these stubs nft as well as dig. A sandbox's own nft answers for a table that is not there, so without it every name would look stranded and the failure would be about the test environment rather than the code.

runtime.bats was not run locally. It is where this earns its keep: every container that suite boots now asserts the property for every permitted entry, so a future class or wiring mistake fails the privileged job instead of shipping.

Refs #350

**Summary generated by LLM agent** Closes the gap that four review rounds kept falling into, by asserting the property in a running container instead of inferring it from reading. ## What was only checked for one destination Check 4 asserted that a permitted name resolves *and* its address reaches the address set its class names. It did that for exactly one host: the one in the GitHub metadata URL, which is the destination hardest to get wrong. The operator's own forge, the Loki host, a self-served backend and every approved domain were never subjected to it. That is where the defects have been. The class an entry was recorded with, the resolver line built from it, and the set the ports are keyed to are three separate statements, and each can be right about a different set. Reading the code cannot tell them apart, which is why every defect of this shape reached a green suite: - a forge name recorded in the open class, so its answers fed the port-unrestricted set - a forge name skipped out of every class, so `internal_domains` could not find it - a name that could not resolve at all, because the resolver will not forward a dotless query All three present at boot in the same way: a name that answers, and an address that is not where the ports expect it. One check catches all three, in the real container, before the session starts. ## The change The sweep that already resolves every permitted name now reads each answer back out of its class's set. It is fatal, unlike a name that returns no address. That distinction is deliberate and kept: an unresolvable name is the world being unavailable, and refusing to start a session over a host that happens to be down would cost more than it prevents. A name that resolves to an address the policy did not permit in the class it was recorded under is the policy disagreeing with itself, and the session it produces reaches a configured destination only by accident, or not at all. Which set a class feeds now has **one** definition. The config that fills a set and the check that reads it back asking separately is the same shape as the bug: they could name different sets and the check would pass while the class meant nothing. ## One name, one class Writing the check found that this repository's own allowlist would fail it. A name can reach both class lists: `release-assets.githubusercontent.com` is an open-class entry of the component's own and also sits under a forge suffix, so the approved-domains loop adds it to the forge class as well. Any harness or backend domain under a forge suffix would do the same. That produced two entries for one name, two resolver lines naming different sets, and one of them honoured - so nothing could say which class was in force, and the new check would look in the set that lost and stop the launch. Entries are now deduplicated by name, keeping the first, and forge is recorded first. A name that is forge class by any route keeps the port restriction rather than inheriting the open class from a second mention. That also makes `fixed-domains.sh`'s claim that the forge class wins wherever the two overlap true of the entries file, which it was not. This is the item recorded as the third finding on #350. ## Testing Full confined gate, all ten suites, green. The new assertions were each run against their own removal: dropping the membership check lets a stranded name pass, giving the two places their own set name defeats the single-definition test, and removing the deduplication produces two entries for the one name. The stranded case is covered in both shapes - an address in no set at all, and a forge-class name whose address is in the open class's set, which is the one that actually happened and which a check asking only "is it permitted at all" would miss. The harness for these stubs `nft` as well as `dig`. A sandbox's own `nft` answers for a table that is not there, so without it every name would look stranded and the failure would be about the test environment rather than the code. `runtime.bats` was not run locally. It is where this earns its keep: every container that suite boots now asserts the property for every permitted entry, so a future class or wiring mistake fails the privileged job instead of shipping. Refs #350

dejima-agent added 2 commits 2026-09-14 23:14:32 -07:00

Assert every permitted name reaches the set its class names ... 5ee405fd6e

Check 4 asserted this for one destination: the host of the GitHub metadata
URL, which is the one hardest to get wrong. The operator's own forge, Loki, a
self-served backend and every approved domain were never subjected to it.

That is the gap four review rounds kept falling into. The class an entry was
recorded with, the resolver line built from it, and the set the ports are
keyed to are three separate statements, and each can be right about a
different set. Reading the code cannot tell them apart, and every defect of
that shape here reached a green suite: a forge name recorded in the open
class, a name skipped out of every class, a name that could not resolve at
all. All three present at boot as a name that answers and an address that is
not where the ports expect it.

The sweep that already resolves every permitted name now reads each answer
back out of its class's set. Fatal, unlike a name that did not answer: that
one is the world being unavailable and deliberately costs nothing, while this
one is the policy disagreeing with itself.

Which set a class feeds now has one definition. The config that fills a set
and the check that reads it back asking separately is the same shape as the
bug, and would have let the check pass while the class meant nothing.

The harness for these tests stubs nft as well as dig, because a sandbox's own
nft answers for a table that is not there - every name would look stranded
and the failure would be about the test environment.

Record a name once, in the narrower class, so its entry can be checked ... 9764ddd4fc

A name can reach both class lists. release-assets.githubusercontent.com is an
open-class entry of this component's own and also sits under a forge suffix,
and any harness or backend domain under one would do the same - so this
repository's own allowlist produces it.

Two lines meant two resolver lines naming different sets for one name, of
which the resolver honours one. The entries file and `status` could not say
which class was in force, and the check added alongside this - which reads an
answer back out of its class's set - would look in the set that lost and stop
the launch. That is how this was found: the check refused the project's own
configuration.

Forge is recorded first and the deduplication keeps the first occurrence, so
a name that is forge class by any route keeps the port restriction rather
than inheriting the open class from a second mention. That also makes
fixed-domains.sh's claim that the forge class wins wherever the two overlap
true of the entries, which it was not.

dejima-agent added 1 commit 2026-09-15 00:08:09 -07:00

Handle the empty answer the policy's own set -e turns into an abort ... c805208fac

resolve_one returns non-zero for a name that answered with no address, which
is an ordinary outcome. The sweep assigned from it bare, and under
`set -Eeuo pipefail` that aborts - so a permitted name whose host is
unreachable took the container down, which is the opposite of what the note
two lines below it exists to do. The previous form tested the substitution
inside `[ -n ... ]`, where a failure is part of a condition and exempt.

The harness could not see it. bats runs a test with errexit off, and `set -e`
inside a subshell of that shell does not restore it - measured rather than
assumed, because the first attempt at this fix was exactly that subshell and
it reported the regression as passing. The extracted functions now run in a
real child bash with the options egress-policy sets, which reproduces the
failure and then the fix.

It also wanted a table name the harness had never supplied, which the
stricter shell surfaced immediately as an unbound variable. Read out of the
script, like the exemption hint beside it.

Covered by the case that reaches it: a lookup that succeeds and carries no A
record. The existing empty-answer test stubs a dig that fails, and resolve_one
returns zero on that path, so it never reached the abort.

branderson approved these changes 2026-09-15 19:46:55 -07:00

branderson merged commit 09d1881633 into development 2026-09-15 19:47:09 -07:00

dejima-agent referenced this pull request 2026-09-16 00:00:36 -07:00

Release: the egress filter and the container image, both rewritten clean-room #347

dejima-agent referenced this pull request 2026-09-16 00:23:52 -07:00

Finish the release's documentation review, and two things it found in code #359

Sign in to join this conversation.

Reviewers

No Reviewers

branderson

Labels

Clear labels

Compat/Breaking

Breaking change that won't be backward compatible

Kind/Bug

Something is not working

Kind/Documentation

Documentation changes

Kind/Enhancement

Improve existing functionality

Kind/Feature

New functionality

Kind/Security

This is security issue

Kind/Testing

Issue or pull request related to testing

Priority

Critical

1

The priority is critical

Priority

High

2

The priority is high

Priority

Low

4

The priority is low

Priority

Medium

3

The priority is medium

Reviewed

Confirmed

1

Issue has been confirmed

Reviewed

Duplicate

2

This issue or pull request already exists

Reviewed

Invalid

3

Invalid issue

Reviewed

Won't Fix

3

This issue won't be fixed

Status

Abandoned

3

Somebody has started to work on this but abandoned work

Status

Blocked

1

Something is blocking this issue or pull request

Status

Need More Info

2

Feedback is required to reproduce issue or to continue work

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

branderson dejima-agent

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dejima/dejima#356