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)