Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,13 @@ covered by a test, and none of them is enforced by the platform.
container output is stored with the execution and searchable. Route every message that
can reach a log through the redaction helper — an HTTP library puts the whole URL,
signature included, into the text of its errors.
10. **Run as uid 10001.** The agent bind-mounts the credentials directory mode `0700`
owned by its own uid; a mismatch is "permission denied" on the step's own credentials
and nothing in the platform warns about it. Do not "fix" it by running as root.
10. **Run as a non-root user — any of them.** No uid has to match the agent's. The
credentials directory is mounted `0711` with the file inside it `0444`, so any user
can open it; confidentiality comes from an ancestor directory nobody else can
traverse. What the modes *do* still require: open the exact path in
`LSPO_CREDENTIALS_FILE`, never list its directory — `0711` grants traversal, not
enumeration. This rule used to say "run as uid 10001"; that was true of an older
platform and telling an author to build for it is now the harmful answer.

## House style

Expand Down
74 changes: 71 additions & 3 deletions CONFORMANCE-BASELINE.md
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,63 @@ from the sentence to the conclusion. That is a judgement about meaning, no strin
answers it, and it stays with whoever reviews the test. The structural check makes the
judgement *possible* by forcing the sentence into the open where a reader can weigh it.

### The uid instruction that was false — the check earning itself a second time

The same check fired again, on the worst thing a public repository can be wrong about: an
instruction telling authors how to *build* their image, where following it is what hurts
them. This repository said, in nine places, that a job's credentials arrive as a `0600`
file in a `0700` directory owned by the agent's uid, and that a customer's image therefore
had to run as **uid 10001**. Every part of that had stopped being true.

* The credentials **file** is `0444` and its directory `0711`, re-applied on every write
(orchestrator PR #250). Confidentiality comes from an ancestor directory nobody else can
traverse, not from the leaf's mode — precisely so that an image running as any uid can
open its own credentials.
* The **agent** now runs as the operator's own account, not as the account its image
declares (orchestrator PR #268, `--user "$(id -u):$(id -g)"`). So "the agent is 10001"
was not what a deployed agent gave you either.

The two errors compounded in the nastiest possible way. A half-correction that fixed only
the second — "the agent runs as the invoking user, so build as uid 1000" — would have been
*worse* than the original text, because it keeps the false `0700`/`0600` premise alive and
sends the author chasing a number that changes per machine. The premise had to go first.

What the citation check actually caught was one stale quotation: the old permissions test
cited *"The directory is created 0700 and the file 0600 — on a shared machine the credential
must not be readable by other users"*, and `agent/creds.py` no longer contains that
sentence. One red assertion, on one test, was the only thread that led to nine wrong
statements across the documents, the Dockerfile and the harness's own constants — none of
which any test would have contradicted, because they were prose.

The test it guarded has been replaced rather than deleted, and the replacement is stronger
than the original: instead of asserting a coupling, it now measures that the coupling is
**gone**, with real containers and four different users. See
`test_a_workload_running_as_any_uid_can_read_its_own_credentials`.

**Its liveness was measured, not assumed**, because a test that says "everything is
readable" is exactly the shape that passes when nothing is being checked. Four mutations,
each run against real containers, and each has to fail for the *right* reason:

| Mutation | Result |
|---|---|
| the credential file narrowed back to `0600` | red — *"a 0600 file in a 0711 directory owned by uid 1000 was NOT readable as the image's own user"* |
| the directory narrowed back to `0700` | red — the same, naming `0700` |
| the directory widened to `0755`, so it can be listed | red — *"a 0755 directory was listable by a uid that does not own it"* |
| the mode re-applied at job start but **not** on the refreshed inode | red — *"the replaced credential file was not readable by an arbitrary uid"* |

And two negative controls, which must stay **green**, because the whole claim is that the
image's own uid is nobody's business: rebuilding this repository's image as uid `10001`
(the agent image's account) and as uid `1000` (a typical host operator) both pass.

The first draft of the test failed that battery in the most instructive way. It opened with
`assert CREDENTIALS_FILE_MODE == 0o444` — and narrowing the constant then failed on *that*
line, comparing a literal in this repository against a constant in this repository, with
the container never starting. A guard that marks its own homework. Removing it is what
turned the mutations into the four honest failures above. The lesson is the same one this
section already teaches, sharpened: a quotation is the only part of a document that can be
mechanically held to its source, so the rules worth quoting are the ones a reader will act
on — and a check must compare itself against something it does not also own.

---

## One demotion I would argue about — and the sentence that would settle it
Expand Down Expand Up @@ -654,8 +711,12 @@ begin-after-expiry case becomes worth splitting out as a test of its own.
`tests/test_hello_node_example.py` does exercise that branch.)
* **The agent's real credential-file permissions, end to end.** Measured in
`tests/test_platform_rules.py` with a synthetic directory rather than one a running
agent produced. The harness itself deliberately uses 0755/0644 everywhere else, so that
a permission problem can never be mistaken for a node defect.
agent produced. The harness now applies the agent's real modes everywhere — `0711` on
the directory, `0444` on the file — rather than the looser 0755/0644 it used while
those modes were still 0700/0600 and would have locked the harness out of its own
fixture. That change is worth more than tidiness: the directory belongs to whoever ran
pytest and the container runs as somebody else, so every container test in the suite
now reads its credentials through the same permission class a customer's image uses.
* **The environment allowlist end-to-end.** The agent refuses a manifest naming a variable
outside `LSPO_AGENT_ALLOWED_ENV` *before the container starts*, so no black-box test of
the node can observe it.
Expand All @@ -667,7 +728,14 @@ begin-after-expiry case becomes worth splitting out as a test of its own.
the platform reworded while the baseline was being measured (see "The check earned itself
this round"). It catches a rule whose WORDS changed; a rule whose words stayed and whose
behaviour changed would still pass. Two tests really do exercise the mechanism with real
containers: the bind-mounted-file test and the 0700-permissions test.
containers: the bind-mounted-file test and
`test_a_workload_running_as_any_uid_can_read_its_own_credentials`, which reads a job's
credentials from inside the image as three users — the image's own, a uid in no passwd
file, and the directory's owner — then once more as that stranger uid across a
credential refresh, and finally confirms it still cannot LIST the directory. The citation check
fired a second time on exactly this area: the platform had widened those modes so that a
node image may run as any user, and this repository was still instructing authors to
build as uid 10001 (see "The uid instruction that was false").
* **Generation fencing.** That a superseded runner physically cannot write into the live
attempt's directory is a property of the staging prefix and the upload policy, not of the
node. The harness proves the fence exists (a key outside the prefix is refused with 403)
Expand Down
24 changes: 16 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,22 @@ FROM python:3.12-slim
# more than that. A container that runs as root is a container that can do more damage
# than the job it was given.
#
# The NUMBER matters, not just the fact of being non-root. The agent bind-mounts each
# job's credentials directory mode 0700, owned by the uid the AGENT process runs as, and
# a 0700 directory owned by uid A is unreadable by a process running as uid B. Nothing in
# the platform compares the two or warns; it surfaces as "permission denied" on the
# step's own credentials file and looks like a broken node. 10001 is what the shipped
# agent image uses. An agent started directly on a host instead runs as the invoking
# user, usually 1000 — rebuild with `--build-arg STEP_UID=1000` if that is yours.
ARG STEP_UID=10001
# The NUMBER does not matter, and this file used to say the opposite. It instructed you
# to build as uid 10001 because the agent bind-mounted each job's credentials directory
# mode 0700 with the file inside it 0600, owned by the uid the agent ran as — so an image
# declaring any other user could not open its own credentials. That was a real defect and
# it has been fixed on the platform, in both of its halves: the mounted directory is now
# 0711 and the file inside it 0444, re-applied on every write, so any uid can open a path
# it has been told the name of; and the agent is started as the operator's own account
# (`docker run --user "$(id -u):$(id -g)"`) rather than as the uid its own image declares,
# so "the agent is 10001" is not true of a deployed agent either.
#
# So 4242 here is arbitrary, and deliberately NOT the agent image's 10001 — a number this
# file shares with the agent is a number the next reader will assume has to match. Pick
# whatever suits you, or set --build-arg STEP_UID=<n>. The one thing that is still true of
# the mode bits: the credentials directory is traversable but not LISTABLE by you, so open
# the exact path in LSPO_CREDENTIALS_FILE and never enumerate the directory it sits in.
ARG STEP_UID=4242
RUN useradd --create-home --uid ${STEP_UID} step

WORKDIR /app
Expand Down
6 changes: 5 additions & 1 deletion conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ orchestrator, so a change there cannot turn them red by itself. Their value is t
rules the rest of the suite leans on are written down with a citation a human can check in
one step — which is exactly how the "the agent injects exactly nine variables" error in
this file was found. Two of them are not restatements and really do exercise the
mechanism, with real containers: the bind-mounted-file test and the 0700-permissions test.
mechanism, with real containers: the bind-mounted-file test and the one that reads a
job's credentials from inside the image as four different users, including a uid that
exists in no passwd file. That second one is the closest this repository comes to a
tripwire on the platform — it goes red if the credential modes are ever narrowed back to
the shape that forced a customer's image to run as one particular uid.

## What is in here

Expand Down
35 changes: 25 additions & 10 deletions conformance/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,32 @@
#: The opt-in progress line prefix. The trailing space is part of it.
PROGRESS_PREFIX = '@lspo:progress '

#: The uid the orchestrator's agent runs as inside its own container, and therefore the
#: OWNER of every credentials directory it writes. The directory is 0700 and the file
#: 0600, so a workload image running as any other non-root user cannot read its own
#: credentials. Written down here because it is a real constraint on a customer's
#: Dockerfile that nothing in the contract documents states — see
#: The mode the agent gives a job's credentials DIRECTORY (``agent/creds.py``
#: ``CREDS_DIR_MODE``): **traversable by everybody, listable by nobody but the agent**.
#: The execute bit is what lets a KNOWN filename inside be opened; the read bit is what
#: lets the directory be enumerated, and the contract never needs that — your container
#: is told the exact path in ``LSPO_CREDENTIALS_FILE``. So this is the one real
#: constraint the mode bits still place on a node: open the path you were given, do not
#: list the directory it is in.
CREDENTIALS_DIR_MODE = 0o711

#: The mode the agent gives the credentials FILE itself (``agent/creds.py``
#: ``CREDS_FILE_MODE``): **readable by every uid, writable by none**, and re-applied on
#: every write, a credential refresh included. What keeps the credential off the rest of
#: the machine is the agent's own working directory ABOVE the mounted leaf — owner-only,
#: bind-mounted into nothing — rather than this file's mode.
#:
#: **There is therefore no uid your image has to run as**, and this pair of constants is
#: the whole of the reason. There used to be: the directory was 0700 and the file 0600,
#: owned by the uid the agent happened to run as, so an image declaring any other user
#: could not open its own credentials and the only repair a customer could find was to
#: run their container as root — the platform punishing the careful choice. Both halves
#: of that are gone (orchestrator PR #250 for the modes; PR #268 makes the agent run as
#: the operator's own account rather than as the uid its image declares), and this
#: repository no longer records an agent uid at all, because a number written down here
#: is a number somebody will build an image around. See
#: ``tests/test_platform_rules.py`` and CONFORMANCE-BASELINE.md.
AGENT_UID = 10001

#: The mode the agent gives a job's credentials directory and file.
CREDENTIALS_DIR_MODE = 0o700
CREDENTIALS_FILE_MODE = 0o600
CREDENTIALS_FILE_MODE = 0o444

SHA256_PATTERN = re.compile(r'[0-9a-f]{64}')

Expand Down
27 changes: 19 additions & 8 deletions conformance/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,16 @@ def __init__(

self.creds_dir = workdir / 'creds'
self.creds_dir.mkdir(parents=True, exist_ok=True)
# 0755/0644, not the agent's 0700/0600: this image runs as uid 10001 and the
# harness runs as whoever invoked pytest, so the agent's own permissions would
# make the file unreadable here for a reason that has nothing to do with the
# node. See CONFORMANCE-BASELINE.md — whether the real agent hits the same wall
# is a platform question, not a node one.
os.chmod(self.creds_dir, 0o755)
# Exactly the modes the real agent applies (``contract.CREDENTIALS_*_MODE``),
# rather than the looser 0755/0644 this used to use. The looser pair existed
# because the agent's own modes at the time — 0700 on the directory, 0600 on the
# file, owned by the agent's uid — would have made the file unreadable to this
# image for a reason that had nothing to do with the node. That is no longer
# true, and using the real modes buys something: this directory is owned by
# whoever invoked pytest and the container runs as somebody else entirely, so
# EVERY container test in this suite now reads its credentials the way a
# customer's image does in production, through the "other" permission class.
os.chmod(self.creds_dir, contract.CREDENTIALS_DIR_MODE)

self.endpoint = Endpoint(max_object_bytes=max_object_bytes)
self.endpoint.on_rotate = self._write_creds
Expand Down Expand Up @@ -259,7 +263,10 @@ def _write_creds(self, token: str) -> None:
data = json.dumps(self.envelope(token), sort_keys=True, indent=2).encode('utf-8')
handle, tmp = tempfile.mkstemp(dir=str(self.creds_dir), prefix='.creds-', suffix='.json')
try:
os.fchmod(handle, 0o644)
# On the NEW inode, every time, because that is what the agent does. A mode
# applied once and not re-applied would let every short test pass and fail
# only the jobs that live long enough to see a refresh.
os.fchmod(handle, contract.CREDENTIALS_FILE_MODE)
with os.fdopen(handle, 'wb') as stream:
stream.write(data)
stream.flush()
Expand Down Expand Up @@ -288,8 +295,12 @@ def write_envelope_file(self, filename: str, token: str) -> Path:
node believes.
"""
path = self.creds_dir / filename
# Unlinked first because the mode below leaves the file read-only to everybody,
# its owner included: a second call under the same name would otherwise fail on
# the write rather than on anything the test is about.
path.unlink(missing_ok=True)
path.write_bytes(json.dumps(self.envelope(token), sort_keys=True, indent=2).encode('utf-8'))
os.chmod(path, 0o644)
os.chmod(path, contract.CREDENTIALS_FILE_MODE)
return path

def start(
Expand Down
22 changes: 16 additions & 6 deletions docs/AUTHORING.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ register a repo digest before a second runner joins.
"/app/node.py"]`, so your process really is the container's PID 1 and receives signals
directly rather than through a shell that will not forward them.

**RECOMMENDATION.** Run as uid 10001 unless the agent's operator tells you otherwise; see
[PROTOCOL.md](PROTOCOL.md#13-where-the-credentials-live-and-who-may-read-them) for why
that number, and why "run as root" is the wrong repair.
**RECOMMENDATION.** Run as a non-root user — **any** non-root user. No uid has to match
the agent's: your credentials file is mode `0444` in a `0711` directory, so any user can
open it. This document used to name a number here, and that instruction is withdrawn; see
[PROTOCOL.md](PROTOCOL.md#13-where-the-credentials-live-and-who-may-read-them) for what
changed and why "run as root" was, and still is, the wrong repair.

**RECOMMENDATION.** Set `PYTHONUNBUFFERED=1`, or your language's equivalent. Without it a
buffered stdout means your logs arrive only when the process ends, which is exactly when
Expand Down Expand Up @@ -408,6 +410,12 @@ apart either treats advice as law or treats law as advice. Both are expensive.
* [ ] **RECOMMENDATION.** Reads the credentials path from `LSPO_CREDENTIALS_FILE`, with no
fallback that hides a missing variable. Nothing checks how you find the path; there
is simply nothing else to read.
* [ ] **RULE, enforced by the kernel rather than by a check on your node.** OPENS that
path. Does **not** list the directory it is in to discover the file: the agent
mounts that directory `0711`, which grants traversal but not enumeration, so a
listing is a permission error for every user except the agent. You were given the
name, so nothing needs the listing
([PROTOCOL.md](PROTOCOL.md#13-where-the-credentials-live-and-who-may-read-them)).
* [ ] **RECOMMENDATION.** Refuses an envelope whose `schema_version` it does not
implement, and a `scheme` or `staging.mode` it does not support.
* [ ] **RECOMMENDATION.** Ignores envelope and manifest fields it does not recognise,
Expand Down Expand Up @@ -524,6 +532,8 @@ apart either treats advice as law or treats law as advice. Both are expensive.

* [ ] **RULE.** Pinned by digest — `registry/name@sha256:<64 hex>` or a bare
`sha256:<64 hex>`. A tag is refused at registration.
* [ ] **RECOMMENDATION.** Exec-form entrypoint, unbuffered output, and a uid matching the
agent's (10001 for the shipped agent image). The uid is checked by nothing and
fails as a permission error on your own credentials file.
* [ ] **RECOMMENDATION.** Exec-form entrypoint, unbuffered output, and a non-root user of
your own choosing. **No particular uid is required** — the credentials file is
`0444` in a `0711` directory, so any user can open it. An earlier version of this
checklist demanded a uid matching the agent's; that is withdrawn
([PROTOCOL.md](PROTOCOL.md#13-where-the-credentials-live-and-who-may-read-them)).
Loading
Loading