From 6f3c936071fc06896b6c3cd41e49d6f81304ac50 Mon Sep 17 00:00:00 2001 From: makseq Date: Tue, 11 Aug 2026 01:50:01 +0300 Subject: [PATCH] =?UTF-8?q?docs:=20no=20uid=20coupling=20=E2=80=94=20the?= =?UTF-8?q?=20instruction=20to=20build=20as=2010001=20was=20false,=20and?= =?UTF-8?q?=20following=20it=20is=20what=20hurt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This repository told node authors, 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, and it is the worst kind of false: a public document telling somebody how to build their image, where obeying it is the thing that breaks them. Both halves changed on the platform, and both had to be corrected together. `agent/creds.py` now makes the credential FILE `0444` inside a `0711` directory — traversable by anyone, listable by nobody but the agent — and re-applies both on every write, refreshes included (orchestrator #250). Confidentiality comes from the agent's own workdir ABOVE the mounted leaf, which is owner-only and bind-mounted nowhere; the leaf is open precisely so that an image running as ANY user can open its own credentials. That shape exists to remove the exact defect this repository was still documenting as a requirement: an image declaring any other user got a permission error on its own credentials, and the only repair a customer could find was to run their container as root — the platform punishing the careful choice. Separately, the agent is started `--user "$(id -u):$(id -g)"` (orchestrator #268), so it runs as the operator, not as the account its image declares. "The agent is 10001" was not what a deployed agent gave you either. Correcting only the second — "the agent is the invoking user, so build as uid 1000" — would have been WORSE than the original text: 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, so it did, everywhere: OPERATIONS.md (section retitled, with what it replaced stated rather than quietly deleted), PROTOCOL.md §1.3, AUTHORING.md's step 3 and its checklist, CLAUDE.md rule 10, the Dockerfile, CONFORMANCE.md's level-3 claim, docs/README.md's routing row, and the harness's own constants. What replaces it is a real constraint rather than nothing. `0711` grants traversal, not enumeration: open the exact path in `LSPO_CREDENTIALS_FILE`, never list the directory it sits in. That is labelled RULE with the qualifier that the kernel enforces it, not a check on your node. `conformance/contract.py` loses `AGENT_UID` entirely — a number written down as "the agent's uid" is a number somebody builds an image around — and carries the two real modes instead. `conformance/job.py` now applies those real modes to its fixture rather than the looser 0755/0644 it used while the agent's own modes would have locked the harness out. That buys something: 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 image itself moves to an arbitrary uid 4242, deliberately NOT the agent image's 10001, because a number shared with the agent is a number the next reader assumes has to match. The platform test is replaced rather than deleted, and the replacement is stronger than the original: instead of asserting the coupling, it measures that the coupling is GONE. `test_a_workload_running_as_any_uid_can_read_its_own_credentials` reads a job's credentials from inside the image as three users — the image's own, a uid in no passwd file anywhere, and the directory's owner — then again as that stranger across an atomic replacement, and finally confirms the stranger still cannot LIST the directory. Real containers; the kernel answers. Its liveness was measured, not assumed. Four mutations, each red for the right reason: file narrowed to 0600, directory narrowed to 0700, directory widened to 0755 so it can be listed, and the mode applied at job start but not to the refreshed inode. Two negative controls stay green — rebuilding this image as uid 10001 and as uid 1000 — because the whole claim is that the image's uid is nobody's business. The first draft FAILED that battery instructively: it opened with `assert CREDENTIALS_FILE_MODE == 0o444`, so narrowing the constant failed on a literal in this repository disagreeing with a constant in this repository and the container never started. A guard marking its own homework. Removing it is what turned the mutations into honest failures. How this was found: the repository's own verbatim-citation check, pointed at the orchestrator, went red on the one quotation that had gone stale — "The directory is created 0700 and the file 0600 — on a shared machine the credential must not be readable by other users", a sentence `agent/creds.py` no longer contains. One red assertion was the only thread leading to nine wrong statements, none of which any test could have contradicted, because they were prose. Recorded in CONFORMANCE-BASELINE.md as the second time that check earned itself. Verified against `origin/master` at 29341bd0: `python -m pytest` (what CI runs) 128 passed, 1 skipped; and with LSPO_ORCHESTRATOR_SRC/REF set, 129 passed with the citation check green. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 10 ++- CONFORMANCE-BASELINE.md | 74 ++++++++++++++++++- Dockerfile | 24 ++++--- conformance/README.md | 6 +- conformance/contract.py | 35 ++++++--- conformance/job.py | 27 ++++--- docs/AUTHORING.md | 22 ++++-- docs/CONFORMANCE.md | 7 +- docs/OPERATIONS.md | 70 ++++++++++++------ docs/PROTOCOL.md | 74 +++++++++++-------- docs/README.md | 2 +- tests/test_platform_rules.py | 133 ++++++++++++++++++++++++++--------- 12 files changed, 360 insertions(+), 124 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9c34b01..ceb375e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 diff --git a/CONFORMANCE-BASELINE.md b/CONFORMANCE-BASELINE.md index b2494f6..4ede471 100644 --- a/CONFORMANCE-BASELINE.md +++ b/CONFORMANCE-BASELINE.md @@ -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 @@ -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. @@ -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) diff --git a/Dockerfile b/Dockerfile index 72f2d7d..595b46a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -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=. 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 diff --git a/conformance/README.md b/conformance/README.md index 699cdf0..5575697 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -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 diff --git a/conformance/contract.py b/conformance/contract.py index 2a46ee0..8f43313 100644 --- a/conformance/contract.py +++ b/conformance/contract.py @@ -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}') diff --git a/conformance/job.py b/conformance/job.py index 140d2cd..cf8e9bd 100644 --- a/conformance/job.py +++ b/conformance/job.py @@ -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 @@ -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() @@ -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( diff --git a/docs/AUTHORING.md b/docs/AUTHORING.md index ff92346..7941d6c 100644 --- a/docs/AUTHORING.md +++ b/docs/AUTHORING.md @@ -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 @@ -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, @@ -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)). diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 6a28a1c..eb5179c 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -429,8 +429,11 @@ that ignores SIGTERM completely. Assert the marker and the timing. **RECOMMENDATION.** Register the node, start an agent, run the pipeline. See [OPERATIONS.md](OPERATIONS.md#registering-a-node). -**What only level 3 proves.** That your image's uid can read its credentials; that the -agent's environment allowlist permits every variable your deployment declares; that your +**What only level 3 proves.** That your container can read the credentials a **real** agent +wrote — which is not a question about your image's uid (any uid can open them) but about +that machine: a state directory on a share that reports permissions without enforcing them, +or an image that makes `/lspo` non-traversable, both fail here and nowhere else. Also that +the agent's environment allowlist permits every variable your deployment declares; that your output ports arrive downstream as the artifact kinds you expected; that collection accepts your hashes. diff --git a/docs/OPERATIONS.md b/docs/OPERATIONS.md index b5cef4d..4c557ea 100644 --- a/docs/OPERATIONS.md +++ b/docs/OPERATIONS.md @@ -268,29 +268,57 @@ ceiling at all (`agent/executors/docker_exec.py:248-274`). --- -## The uid coupling - -**BEHAVIOUR.** Each job's credentials directory is created on the agent's disk with mode -`0700` and the file inside it `0600`, owned by the uid the agent process runs as -(`agent/creds.py:88-98`, `agent/identity.py:64-65`). In object-storage mode the agent does -**not** force your container's user, so your image runs as its own `USER`. - -A `0700` directory owned by uid A is unreadable to a process running as uid B. So the two -uids must match, and nothing checks or warns. - -* The shipped agent image runs as **uid 10001** (`Dockerfile.agent`). -* The example node image also uses **uid 10001**, independently. -* An agent started directly on a host instead of from that image runs as the invoking - user, commonly uid 1000, and then 10001 is wrong. - -**RECOMMENDATION.** Build your image with `USER` at uid 10001 and confirm with whoever -runs the agent. Do not "fix" a permission error by running as root: it does work, because -root bypasses the check, and it puts a root process on somebody's machine for nothing. +## Which user your image runs as + +**BEHAVIOUR.** Your image may run as **any user it likes**, and no uid has to match +anything on the agent's side. Each job's credentials directory is created on the agent's +disk mode `0711` — anyone may walk through it, only the agent may list it — with the file +inside it mode `0444`, readable by every uid and writable by none, and **both modes are +re-applied on every write**, a mid-job credential refresh included +(`agent/creds.py:79-106`, `CREDS_DIR_MODE` / `CREDS_FILE_MODE` / +`JobCredentials.write`). What keeps the credential off the rest of the machine is the +agent's own working directory **above** the mounted leaf, which is owner-only and is +bind-mounted into nothing (`agent/identity.py:74-75`, `ensure_private_workdir`) — not the +file's own mode. + +**BEHAVIOUR.** In object-storage mode the agent does **not** force your container's user, +so your image runs as its own `USER`, and the file is deliberately readable by every uid +so that this stays safe to do (`agent/runner.py` `_local_staging_mounts`). + +**RULE, and it is the only thing the mode bits still ask of you — enforced by the kernel +rather than by any check the platform makes on your node.** Open the exact path the +agent gives you in `LSPO_CREDENTIALS_FILE`. Do **not** list the directory it is in: `0711` +grants traversal, not enumeration, so `os.listdir("/lspo/creds")` is a permission error for +every user except the agent. Nothing needs enumeration — you were told the name. + +**RECOMMENDATION.** Run as a non-root user of your own choosing. Not because a permission +depends on it — none does — but because there is no sandbox around your container, so a +root workload is a root process on somebody else's machine for no benefit. + +### What this replaced, because a document that changed its mind owes you the reason + +Earlier revisions of this page, of `PROTOCOL.md` and of the example `Dockerfile` told you +to build your image as **uid 10001**, and that instruction is now wrong in both of its +halves. Following it is what would hurt you, so it is worth being explicit about what +changed rather than quietly deleting it. + +* **The file was `0600` in a `0700` directory owned by the uid the agent ran as.** An + image declaring any other user got a permission error on its own credentials, and the + only repair a customer could find was to run their container as root — the platform + punishing the careful choice. The modes above replaced that: confidentiality now comes + from an ancestor nobody else can traverse, which is a property the workload's uid cannot + affect. +* **"The agent is uid 10001" was never something you could rely on, and is no longer even + the common case.** That is the account the agent's own image declares, but the command + an operator is given starts the agent with `--user "$(id -u):$(id -g)"`, so a deployed + agent runs as the person who pasted it. Two independent numbers were being treated as + one. **BEHAVIOUR, local demo mode only.** With local-path staging the agent forces your container to its own uid and gid with no supplementary groups, so your image's user is -ignored entirely. Anything that writes under that user's home directory works in -production and fails in the demo. Use `/tmp` or the staging directory. +ignored entirely — the opposite problem, and it is still live. Anything that writes under +that user's home directory works in production and fails in the demo. Use `/tmp` or the +staging directory. --- @@ -555,7 +583,7 @@ two rows where that distinction bites are marked inline. | Agent logs 401 at startup | wrong pool token, or a stale `LSPO_AGENT_TOKEN` still in the environment, which wins over the saved identity | RULE — the token is authenticated on every request | remove the stale variable; the saved identity is enough after the first start | | Job fails immediately naming an environment variable | the deployment declares a variable the agent's `ALLOWED_ENV` does not permit | RULE — checked before your container starts | add the name or a pattern to the agent's allowlist, or stop declaring it | | Container dies at once with a missing credentials file | the agent's state is in a docker volume rather than a host path, so the credentials directory the daemon mounted was an empty one it created | Unchecked — the docker daemon creates an empty directory rather than failing | mount a real host directory at the same path inside and outside, with the workdir a child of it | -| Container dies with permission denied on its credentials | uid mismatch between your image and the agent process | Unchecked — nothing compares the two uids or warns | rebuild with the agent's uid, usually 10001 | +| Container dies with permission denied on its credentials | **not** a uid mismatch — the file is `0444` in a `0711` directory, so any user can open it. Either you listed the directory instead of opening the path (`0711` grants traversal, not enumeration), or your image makes `/lspo` itself non-traversable and has defeated its own mount, or the agent's state lives somewhere its modes are not enforced (a CIFS/SMB share, Docker Desktop file sharing, some NFS exports) | Unchecked in your container; the agent refuses at startup for the cases it can detect | open the exact path in `LSPO_CREDENTIALS_FILE` rather than listing its directory; check nothing in your image narrows `/lspo`. Do **not** rebuild as a particular uid — no uid is required, and do not "fix" it by running as root | | Node fails with "`LSPO_CREDENTIALS` is not set" | your code reads the wrong variable name | Unchecked — the platform sets `LSPO_CREDENTIALS_FILE` and cannot police how you read it | read `LSPO_CREDENTIALS_FILE` | | Run fails with "no completion marker" | your container exited 0 without writing `__lspo_complete.json` | RULE — a marker is required on a successful run | write one before exiting 0. (*RECOMMENDATION, not part of the rule*: write one on your own failure path too, so your `error` and your part-finished inventory survive. It will not save a run an operator stopped while it was still running, nor one whose running container was stopped by the runtime budget — neither of those collects anything today) | | Run fails naming a hash or size mismatch | the object changed after you hashed it, or the marker was written before the upload finished | RULE — every published object is re-read and held to the marker | hash the bytes you actually wrote. (*RECOMMENDATION, not part of the rule*: write the marker last — nothing observes write order, so this failure is the only symptom you will ever see of getting it wrong) | diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 3216352..cfc0704 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -212,39 +212,57 @@ ones you do not: both, at the same time. ### 1.3 Where the credentials live, and who may read them -**BEHAVIOUR.** The agent creates one directory per job on its own disk, mode `0700`, with -the credentials file inside it at mode `0600`, and bind-mounts that directory read-only -into your container (`agent/creds.py:88-98`, `agent/identity.py:64-65`). The directory is -owned by the numeric uid the agent process runs as. +**BEHAVIOUR.** The agent creates one directory per job on its own disk and bind-mounts it +read-only into your container. The directory is mode `0711` — traversable by anyone, +listable by nobody but the agent — and the credentials file inside it is mode `0444`, +readable by every uid and writable by none. **Both are re-applied on every write**, and +that matters more than it sounds: a refresh replaces the file with a brand-new one, so a +permission granted once and not re-granted would let a short job pass and kill a long one +partway through (`agent/creds.py:79-106`, `CREDS_DIR_MODE`, `CREDS_FILE_MODE`, +`JobCredentials.write`). + +**BEHAVIOUR.** What keeps that credential off the rest of the machine is not the file's +mode but the agent's own working directory above the mount, which is owner-only and is +bind-mounted into nothing (`agent/identity.py:74-75`, `ensure_private_workdir`). The +protection is an ancestor nobody else can traverse; the leaf is deliberately open, and it +is open so that your image's user is never the platform's business. **BEHAVIOUR.** In object-storage mode the agent does **not** force your container's user; -your image runs as whatever `USER` it declares (`agent/runner.py:2973-2974` returns an -empty user for anything that is not local-path mode). - -The consequence is a coupling that nothing in the platform manages or checks: a `0700` -directory owned by uid A is unreadable by a process running as uid B. - -**RECOMMENDATION, with a hard consequence.** Your image's numeric uid must equal the -numeric uid the agent process runs as. In the shipped agent image that is **10001** -(`Dockerfile.agent`, `useradd --system --uid 10001 ... lspo`, then `USER lspo`), and it is -the only reason the example node works: `examples/hello-node/Dockerfile` in the -orchestrator independently chose the same number. So **build your image to run as uid -10001**, and check with whoever operates the agent, because an agent started directly on a -host rather than from that image runs as the invoking user, typically uid 1000, and then -10001 is the wrong answer. An image whose uid does not match gets permission denied on its -own credentials file, and the failure looks like a broken node rather than a mismatched -uid. Nothing in the platform detects or warns about this today. - -**RECOMMENDATION.** Do **not** solve this by running as root. Root does bypass the -permission check, so it appears to work, and it is the wrong fix: there is no sandbox -around your container (see [OPERATIONS.md](OPERATIONS.md#residual-limits-stated-plainly)), -so a root workload is a root process on somebody's machine for no benefit. If uid 10001 is -impossible for you, say so to whoever operates the agent rather than escalating privilege. +your image runs as whatever `USER` it declares (`agent/runner.py` `_local_staging_mounts` +returns an empty user for anything that is not local-path mode, and says why: "on object +storage the image's own user is left alone, because the only host path it touches is its +own credentials directory, and that one is deliberately readable by every uid so this +stays true"). + +**So there is no uid coupling: build your image to run as whatever user suits it.** Earlier +revisions of this document said the opposite — that your numeric uid had to equal the +agent's, "usually 10001". That was true of an older platform and is now false twice over. +The modes were widened precisely so it would stop being true, and the agent is started with +`--user "$(id -u):$(id -g)"`, so a deployed agent runs as the operator rather than as the +account its own image declares. If you built an image around 10001 on the strength of the +old text, nothing breaks — 10001 is as valid as any other number — but you are free of it. + +**RULE, and the one thing the modes still ask of you — enforced by the kernel rather +than by a check on your node.** Open the exact path named by +`LSPO_CREDENTIALS_FILE`; never enumerate the directory it lives in. `0711` grants +traversal, not listing, so a directory listing is a permission error for every user but the +agent. You were told the name, so nothing needs the listing. + +**RECOMMENDATION.** Run as a non-root user. Not for a permission — none requires it — but +because there is no sandbox around your container (see +[OPERATIONS.md](OPERATIONS.md#residual-limits-stated-plainly)), so a root workload is a +root process on somebody's machine for no benefit. + +**BEHAVIOUR to know about, not a duty on you.** Every process inside your container can +read your job's credentials; they already share a filesystem and an environment, and the +envelope is scoped to that attempt's own staging prefix and expires. And an image that +makes `/lspo` non-traversable defeats its own mount — nothing on the host prevents that. **BEHAVIOUR, local demo mode only.** When the staging area is a local directory the agent forces your container to the agent process's own uid and gid, with **no supplementary -groups** (`agent/runner.py:2991`). Your image's own user is ignored, so anything that -depends on it, most obviously writing under that user's home directory, works in +groups** (`agent/runner.py` `_local_staging_mounts`). Your image's own user is ignored +there — the opposite problem from the one above, and this one is still live — so anything +that depends on it, most obviously writing under that user's home directory, works in production and fails in the demo. **RECOMMENDATION.** Write scratch files to `/tmp` or into your staging area, never into diff --git a/docs/README.md b/docs/README.md index b3d1570..a4a8172 100644 --- a/docs/README.md +++ b/docs/README.md @@ -117,7 +117,7 @@ own label. | How to test my node without an orchestrator | [CONFORMANCE.md](CONFORMANCE.md#level-1-run-it-with-a-hand-written-envelope) | | What a conformance suite can and cannot prove | [CONFORMANCE.md](CONFORMANCE.md#what-testing-cannot-prove) | | How to register my node and start the agent | [OPERATIONS.md](OPERATIONS.md#registering-a-node) | -| Which uid my image must use, and why | [OPERATIONS.md](OPERATIONS.md#the-uid-coupling) | +| Which user my image should run as (any of them — here is why) | [OPERATIONS.md](OPERATIONS.md#which-user-your-image-runs-as) | | Limits, quotas, and things that do not exist yet | [OPERATIONS.md](OPERATIONS.md#residual-limits-stated-plainly) | | Why my run is stuck at "Waiting for runner" | [OPERATIONS.md](OPERATIONS.md#troubleshooting) | diff --git a/tests/test_platform_rules.py b/tests/test_platform_rules.py index 0153806..1a51c40 100644 --- a/tests/test_platform_rules.py +++ b/tests/test_platform_rules.py @@ -14,9 +14,12 @@ restatement against the original in one step, which is exactly how the "exactly nine environment variables" mistake in this file was found and fixed; * two of them are not restatements at all. ``test_a_bind_mounted_file_never_sees_a_rotation`` - and ``test_a_0700_credentials_directory_is_unreadable_to_any_other_user`` exercise real + and ``test_a_workload_running_as_any_uid_can_read_its_own_credentials`` exercise real kernel and docker behaviour with real containers, and would genuinely change if the - platform's mount or permission choices did. + platform's mount or permission choices did. The second of those is the closest thing in + this repository 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. """ from __future__ import annotations @@ -270,62 +273,126 @@ def test_a_bind_mounted_file_never_sees_a_rotation(image, workdir): container.remove() -# ------------------------------------------------- who may read the credentials file +# ------------------------------------------- which user may read the credentials file @subject_is_platform @traces_to( - 'agent/creds.py JobCredentials.write: "The directory is created 0700 and the file 0600 — on a shared ' - 'machine the credential must not be readable by other users", with the agent running as its own uid ' - 'and agent/runner.py setting run_as only in local demo mode.' + 'agent/creds.py: "The workload runs as whatever user the customer\'s image declares, which is not this ' + 'agent\'s user and is not ours to choose — so the credential file is readable by any uid" ' + '"in a directory anyone may walk through but only the runner may list"; CREDS_FILE_MODE is "The ' + 'credential file itself: readable by every uid, writable by none" and "Both modes are applied on EVERY ' + 'call, and that is load-bearing rather than tidy". agent/runner.py _local_staging_mounts: "On object ' + 'storage the image\'s own user is left alone, because the only host path it touches is its own ' + 'credentials directory, and that one is deliberately readable by every uid so this stays true".' ) -def test_a_0700_credentials_directory_is_unreadable_to_any_other_user(image, workdir): - """The undocumented constraint a customer's Dockerfile has to satisfy. - - Setup: a credentials directory with the modes the agent really uses — 0700 on the - directory, 0600 on the file — owned by the user running these tests. - Action: read it from inside the node's image twice: once as the image's own user, - once as the directory's owner. - Validate: the first is refused with a permission error; the second succeeds. - - Not a restatement either: this measures real containers against a real 0700 - directory. In production the two happen to line up — the agent runs as uid 10001 and - this node's image also runs as uid 10001 — so the workload can read a directory only - its owner can open. That is a coincidence, not a design. A customer image that picks - any other non-root user gets ``PermissionError`` on its own credentials file, and - nothing in the contract documentation warns them. Running as root avoids it, which is - precisely the wrong thing to encourage. +def test_a_workload_running_as_any_uid_can_read_its_own_credentials(image, workdir): + """There is no uid your image has to run as — measured, not asserted. + + Setup: a credentials directory with the modes the agent really uses — 0711 on the + directory, 0444 on the file — owned by the user running these tests, which + is nobody the node's image has ever heard of. + Action: read the file from inside the node's image as three different users — the + image's own, a uid that exists in no passwd file anywhere, and the uid of + the directory's owner — and then once more as that stranger uid after the + file has been atomically replaced the way a credential refresh replaces it. + Validate: all four reads return the document. Then, separately, that the stranger uid + is refused when it tries to LIST that directory rather than open the file + it was told the name of — which is the one constraint the modes still place + on a node, and the reason this test does not simply assert "everything is + readable". + + Not a restatement: this runs real containers against a real directory and lets the + kernel answer. It replaces a test that asserted the OPPOSITE, and the replacement is + the stronger of the two. The old one measured a genuine defect — the file was 0600 in + a 0700 directory owned by whatever uid the agent ran as, so an image declaring any + other user got ``PermissionError`` on its own credentials, and the workaround a + customer would find was to run as root. Both halves of that are gone: the modes were + widened so the leaf is readable by anybody who is told its name, and the agent is now + started as the operator's own account rather than as the uid its image declares, so + "the agent is uid 10001" is not true of a deployed agent either. + + So this test is a tripwire on the thing that would bring the defect back. If the + platform ever narrows either mode, the customer-visible symptom is a permission error + inside somebody's container hours into a job — and this goes red first, here, naming + the mode that moved. The refresh case is not padding: the modes are applied on every + write and a refresh creates a brand-new inode, so a permission granted once and not + re-applied would pass every short test and kill exactly the long jobs. """ + import json import os + import tempfile - assert contract.CREDENTIALS_DIR_MODE == 0o700 and contract.CREDENTIALS_FILE_MODE == 0o600 - + # NOTE for anyone tempted to add `assert CREDENTIALS_FILE_MODE == 0o444` here. An + # earlier draft did, and it made the test WEAKER: narrowing the constant then failed + # on a literal in this repository disagreeing with a constant in this repository, + # which is a check marking its own homework, and the container never ran at all. The + # modes below are applied and the kernel is asked; a constant that moves the wrong way + # is reported as what a customer would actually see, which is a container that cannot + # open its own credentials. creds_dir = workdir / 'perms' creds_dir.mkdir(parents=True, exist_ok=True) - (creds_dir / 'creds.json').write_text('{"schema_version": 1}') + creds_file = creds_dir / 'creds.json' + creds_file.write_text('{"schema_version": 1}') os.chmod(creds_dir, contract.CREDENTIALS_DIR_MODE) - os.chmod(creds_dir / 'creds.json', contract.CREDENTIALS_FILE_MODE) + os.chmod(creds_file, contract.CREDENTIALS_FILE_MODE) + + # 31337 deliberately has no entry in the image's /etc/passwd and no home directory: + # "any uid" has to mean a uid nobody arranged for, or the claim is about this image's + # own accounts rather than about the platform. + for user in (None, '31337:31337', f'{os.getuid()}:{os.getgid()}'): + whose = user or "the image's own user" + got = _read_creds_as(image, creds_dir, user=user) + assert '"schema_version": 1' in got, ( + f'a {contract.CREDENTIALS_FILE_MODE:04o} file in a {contract.CREDENTIALS_DIR_MODE:04o} directory ' + f'owned by uid {os.getuid()} was NOT readable as {whose}: {got}. That is the uid coupling coming ' + f'back, and it costs a customer their node' + ) - as_image_user = _read_creds_as(image, creds_dir, user=None) - assert 'PermissionError' in as_image_user, ( - f'a 0700 directory owned by uid {os.getuid()} was readable by the image\'s own user: {as_image_user}' + # The refresh: a new inode moved into place, exactly as agent/creds.py does it. The + # mode has to be re-applied on the new file, and nothing but a real replacement can + # show whether it was. + handle, replacement = tempfile.mkstemp(dir=str(creds_dir), prefix='.creds-', suffix='.json') + with os.fdopen(handle, 'w') as stream: + stream.write(json.dumps({'schema_version': 1, 'generation': 'refreshed'})) + os.chmod(replacement, contract.CREDENTIALS_FILE_MODE) + os.replace(replacement, creds_file) + + after_refresh = _read_creds_as(image, creds_dir, user='31337:31337') + assert '"generation": "refreshed"' in after_refresh, ( + f'the replaced credential file was not readable by an arbitrary uid: {after_refresh}. A mode applied ' + f'once and not re-applied fails only the jobs that live long enough to see a refresh' ) - as_owner = _read_creds_as(image, creds_dir, user=str(os.getuid())) - assert '"schema_version": 1' in as_owner, as_owner + # And the limit of the freedom: the directory is traversable, not listable. + listing = _list_creds_as(image, creds_dir, user='31337:31337') + assert 'PermissionError' in listing, ( + f'a {contract.CREDENTIALS_DIR_MODE:04o} directory was listable by a uid that does not own it: ' + f'{listing}. The node must open the path LSPO_CREDENTIALS_FILE names, never enumerate its directory' + ) def _read_creds_as(image: str, creds_dir, user: str | None) -> str: + return _in_the_image(image, creds_dir, user, 'print(open("/lspo/creds/creds.json").read())') + + +def _list_creds_as(image: str, creds_dir, user: str | None) -> str: + return _in_the_image(image, creds_dir, user, 'import os; print(os.listdir("/lspo/creds"))') + + +def _in_the_image(image: str, creds_dir, user: str | None, script: str) -> str: import os + import re container = docker.start( image, - name=f'lspo-conformance-perms-{os.getpid()}-{user or "image"}', + name=f'lspo-conformance-perms-{os.getpid()}-{re.sub(r"[^0-9a-z]+", "-", (user or "image").lower())}' + f'-{"list" if "listdir" in script else "read"}', env={}, creds_dir=creds_dir, user=user, entrypoint='python', - command=('-c', 'print(open("/lspo/creds/creds.json").read())'), + command=('-c', script), ) try: container.wait(timeout=60)