Repository navigation
fix: one odd contract ended fluid docs, and a failed run left a broken catalog - #764
Merged
Merged
Conversation
…lay beside it `fluid docs --src` globbed `contract.fluid.*`, which also matches the overlay candidate `contract.fluid.<env>.yaml` (the loader's `<stem>.<env>.<suffix>`). An overlay got a page and an index row with no id, as if it were a product. `--src` now lists the three names a root contract has (`contract.fluid.yaml`, `.yml` and `.json`) and nothing else. `--files` is the caller's own glob and is unchanged.
…t a dead link `index.html` listed a contract that did not load as a blank row whose id linked to `contract-contract.html`, a page the run never writes. The reason it failed was only in `index.json`. A row with an error now says why in place of the cells it has no values for, links nowhere, and takes no page name, so a contract whose id slugs to `contract` is no longer renamed `contract-2` because a failed row came first. The error text is escaped, and the row is found by the filter on its text.
`fluid docs` opened `index.html` for writing before it rendered it, and streamed `index.json` into an open file with `json.dump`. A run that failed after a good one left `index.json` cut off mid-document (no longer JSON) or `index.html` at 0 bytes: the previous catalog destroyed, though the command exited 1. Only the contract pages followed the render-before-open rule. The index in both forms and every page are now rendered first and written only when all of them have rendered, so a failing run leaves what the last run wrote. The output directory is created at that point too, so a run that fails leaves nothing behind.
… catalog One contract holding a value of an unexpected type made the whole run fail with `docs_failed` and a bare Python message naming no contract: a numeric or boolean `id` (it sorted against strings), `tags` of numbers or null (they were joined as text), a YAML date in `tags` or `metadata.owner` (not JSON), and `exposes` or `consumes` that is not a list (`len(5)`). The page had the same failures for the last two. Spec 10: one bad contract does not abort the catalog. An id sorts as text now, tags are text in the search blob, JSON-less values (a date, a set, a key that is not a string) are written as their text, and `exposes` or `consumes` that is not a list lists none. The index keeps the contract's own values where JSON has a type for them. If a contract still cannot be summarised, it gets an error row naming it, as one that did not load does, and no page.
…run wrote
Rendering before writing did not cover the writes themselves. A contract
whose name held a lone surrogate ("\ud800" in YAML) failed the write of
index.html after open() had already emptied it, so the run ended with a
0-byte index.html. An id of 242 characters or more named a page over the
255 bytes a file name may have, and "File name too long" came after
index.json and index.html were rewritten to link that page: a dead link.
Every file is now encoded with backslashreplace before any is opened, so a
lone surrogate is shown as its \ud800 text and does not end the run. Each
file is written whole to a temp file beside it (O_EXCL; the mode the umask
gives, or the mode of the file it replaces, never mkstemp's 0600), and only
when every one is written do they replace the catalog, pages first and the
index that links them last. A run that fails before then removes its temp
files, and a directory where a file goes is refused before anything is
replaced. A replace swaps the directory entry, so a symlink or a hard link
planted in --out is replaced, not written through.
A slug over 100 characters is cut to 91 and ends in '-' and 8 hex digits of
the SHA-256 of the whole id, the cap and hash fluid split uses for a long
id (pants' safe_filename default, kubespawner's strip_and_hash), so two
long ids that start alike still get a page each.
…alog A column whose sensitivity was a list or a mapping (sensitivity: [pii, financial]) made the PII check look it up in a set, which raised "unhashable type: 'list'" while the page was rendered, and fluid docs failed as a whole with docs_failed: one odd contract ended the catalog. The schema's sensitivityLevel is one string, so only a string is looked up; a list or a mapping marks nothing and the page renders like any other. Both shapes are among the odd values the robustness tests list.
…lists any integer _plain, which makes a contract's values listable, still let four things through. A YAML .nan or .inf was written to index.json as NaN or Infinity, which strict JSON parsers reject. A !!set became str(set), whose order follows the string hash, so two runs over one contract could differ. A key was str(k), so the YAML 1.1 keys NO:, yes: and ~: became "False", "True" and "None" where index.json has always had json's "false", "true" and "null". And an integer of more than the 4300 digits Python turns into text (a YAML hex literal holds one) made the sort, the JSON and the page raise, so fluid docs failed as a whole. A non-finite float is now written as its text, and index.json is dumped with allow_nan=False so none can come back. A set becomes a list sorted by text, then type. A bool, null or float key is spelled as json spells a key, any other as its text. An integer too long to turn into text is shown as "(an integer too long to show)". The page is rendered from the same plain data as its index row, so none of these reaches the page either.
…ked one
--src was globbed as f"{src}/**/contract.fluid.yaml". The glob follows a
symlinked directory with no loop check, so two links to "." in --src made
fluid docs walk without end, and a link out of --src listed the contracts
outside it. And --src went into the pattern unescaped: --src 'my [team]
products' was a character class that matched nothing, so the catalog came
out empty without a word.
--src is now walked with os.walk(followlinks=False): its name is only a
name, a symlinked directory is not followed (it is logged at DEBUG; name
such a product with --files), and a hidden directory is skipped as the glob
skipped it. A root contract that is itself a symlink is listed when the
file it names is inside --src, and a file reached by two paths is listed
once, by its own path. The three root-contract file names, and the sort,
are as they were; --files, an explicit glob, is unchanged.
Rendering before writing moved os.makedirs(--out) after the render, so an --out that cannot hold the catalog (--out /System/x, an --out naming an existing file, or a directory under one) was reported only after every contract had been loaded and rendered: 33 s on a big catalog, for an error known before the first contract was read. --out is now checked before any contract is read, without making it: an --out that exists and is not a directory, or a directory this user cannot write to, or a missing --out whose nearest existing parent is not a writable directory, ends the run at once with docs_failed and a message naming the path. A run that fails while writing also removes the directories it made, so a failed first run leaves no --out behind.
The broad except in _summarize_contract keeps the catalog when a contract's row cannot be made, but it said nothing: the only trace was "not listed:" and the exception's text in the row, and a fault there would hide a contract with no traceback and no way to see it. The logger is now passed to _summarize_contract, and a row that cannot be made is logged at DEBUG as docs_contract_not_listed with the contract's path and the traceback. A test makes one contract's row fail and pins what the claim says: the run exits 0, the contract has a "not listed:" row in index.json and index.html and no page, and the other pages are written.
… left open Mutation testing of the branch left five claims with no test to fail: an error row whose colspan no longer filled the header, an error row without its path cell, an error row the filter could not find, error rows taking a page name again, and --out made before the render. The error row's cells, colspans summed, now must span the header's ten <th>, every one a <td>, with the error in a cell, the path in the last cell of its own, and data-search holding the error and the path. A contract that does not load sorts before one whose id is "contract", and that contract must keep contract-contract.html. A first run whose render fails must leave no --out behind. Each test fails on the mutant it pins.
Three comments in fluid docs said what it does not do. run said a contract with "no row of its own (it did not load)" gets no page, but every contract has a row: it is a row with an error, from a contract that did not load or could not be listed, that has no page. It said a slug "falls back to a path hash"; it falls back to the file name. _slug_for said the same only for an absent id, when an id that is not a string falls back too. Those now say what happens, run's note on rendering before writing names the writer that covers a failed write, and _summarize_contract and _render_contract_html say the values they get are plain data. Comments only; no behaviour changes.
…ever by path A page that replaces a file keeps that file's mode, and the mode was set with os.chmod(temp) after the temp file was opened. Anyone who can write to --out could rename a symlink over the temp name in between, and the chmod followed it: a 0600 secret.key outside --out became 0777 (a real race hit 11 of 30,000 tries). The mode was also taken from any file at the page's name, so a file of another user, or a hard link to one, chose the mode of the page that replaced it. The mode is now set with os.fchmod on the open descriptor, before a byte is written, so a swapped name is never followed (os.chmod by path only where os.fchmod is missing, Windows before 3.13, which never keeps a mode). And a file's mode is kept only when it is a regular file of this user (st_uid == os.getuid()) with no other name (st_nlink == 1); any other gets the mode the umask gives.
…it made it os.makedirs(--out) ran before the cleanup's try, and the directories to take back were worked out beforehand from abspath(--out). So a makedirs that failed part-way left what it had made (--out new1/new2/<300 chars>/site left new1/ and new1/new2/), and with a symlink before a ".." the list named the wrong directories: --out L/../made made made/ beside L's target, abspath() folded it to one beside L, and the made/ that existed was left. The up-front --out check walked the same folded path, so it judged a parent the directory would never be made in. --out is now made one level at a time inside the try, each directory recorded by its real path as soon as it is made, and a failed run removes exactly those, deepest first. The up-front check walks up --out as written, so "L/.." is the directory the OS would make it in, and names that directory by its real path when it refuses.
… files A run killed between writing its temp files and replacing the catalog with them (SIGKILL at os.replace, a power cut) left .<name>.<16 hex>.tmp files in --out, and no later run removed them: they stayed in the served directory for good. Before writing, a run now removes the temp files left for the names it is about to write: regular files of this user named exactly .<name>.<16 hex>.tmp, never a symlink, a file of another user, a name another run's page had, or any other file. It is best effort: an --out that cannot be listed is left as it is. Two runs writing one --out at the same moment were already a race; the later one can now also take the earlier one's temp files, which fails that run without touching its catalog.
The up-front --out check passed "", so --out "$OUT" with OUT unset read and rendered the whole catalog and only then failed, at the first directory it tried to make, with "No such file or directory: ''". An empty --out now ends the run at once with docs_failed and "--out is empty: name the directory to write the catalog to", before any contract is read.
No test failed a replace, so nothing pinned the order the catalog is moved in: a writer that replaced index.json and index.html before the pages passed every test, though a failure at its first page leaves an index linking a page that was never written. And the kept-mode test used 0o640, which the umask (0o022) leaves whole, so it passed with the code that restores a kept mode removed. A replace now fails at each of the five places in turn (EBUSY), and once not at all: the files replaced must be the pages and then index.json and index.html, in that order, every link of the index left must be to a page that is there, and no temp file may be left. The kept-mode test also runs with 0o664, a mode the umask cuts, so only setting it again keeps it. Each fails on the mutant it is there for.
…itself The cleanup of a failed run removed each directory it had made by the real path it recorded right after the mkdir. Whoever can write where --out is made could swap a symlink in between: with --out shared/a/b/site, "a" renamed away right after its mkdir and replaced by a symlink to an empty directory of the user's made the cleanup rmdir that directory, and a symlink swapped in for "a" once the tree was made made it remove the user's empty b/site under the symlink's target. Right after its mkdir, a directory is now lstat'ed and recorded with its device and inode, and only when it is still a directory. Before each rmdir the path is lstat'ed again, and the directory is removed only while it is the very one the run made: same device, same inode. A run that fails without such a swap still takes back everything it made.
…mod is by path Whether a file in --out was this run's (to keep its mode, or to remove a temp file a killed run left) was decided by comparing its owner with the real uid, while every file a process makes belongs to its effective uid: a setuid run took its own catalog for someone else's. The stale-temp sweep also checked "is a regular file" and "whose" on two separate looks at the entry. And _stage kept an os.chmod-by-path fallback for systems without os.fchmod that could never run: a mode is kept only where there are uids, and every such system has os.fchmod. _mine now compares with os.geteuid(), the sweep checks S_ISREG and the owner on one lstat result, and the fallback is gone, so nothing in docs_build sets a mode by path. The tests that stand for another user now change the effective uid, and a new one pins that a different real uid does not make the run's own files someone else's.
… hour The sweep that removes the temp files a killed run left took any temp file of this run's names, so it also took those of another run of the same user writing the same --out at that moment, one that had written its temp files and not yet moved them in: that run then failed at its first replace. A temp file is now removed only when it was last written over an hour ago. A run takes seconds, so a run still at work never has one that old, while a killed run's are still removed by any run an hour on. git prune expires a stale tmp_obj_ file the same way, by its age (gc's default there is two weeks). Tests pin both sides of the hour: 61 minutes goes, 59 minutes and a fresh one stay.
The up-front check walked up --out as written to the nearest directory there is. When a ".." follows a part not made yet, that is not where the directory lands: ro/inner/new/../../x is made in ro, but the walk stopped at ro/inner. With ro read-only and ro/inner writable, the check passed and the run failed only after the whole catalog was loaded and rendered. The check now also walks up from the parent of os.path.realpath(--out), which takes such a ".." where the OS takes it, and both walks must end in a directory this user can write to. The walk as written stays: a realpath alone looks through a dangling symlink (dangling/site) or folds a file away (a-file/../site), and the mkdir fails on both, so they are refused at once as before; a test pins those two cases as well.
# Conflicts: # CHANGELOG.md
github-actions
Bot
added
cli
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…ds no BaseException catch os.open then open(fd) left the descriptor open when open() itself failed (code-quality: file is not always closed). The staged write and the catalog write now clean up in a finally behind a done flag, so an interrupt is still taken back without catching BaseException.
# Conflicts: # CHANGELOG.md
…obustness # Conflicts: # CHANGELOG.md
…no mode literal CodeQL #315 (py/overly-permissive-file) flagged os.open(temp, flags, 0o666): a world-writable literal, although the umask made the file 0o644, as open() does. The temp file is now made by open(temp, "xb"), the same open(2) (O_CREAT | O_EXCL, 0o666 less the umask). A file that replaces this user's own keeps going through os.open with that file's mode, as open()'s opener, so it is never wider than the file it replaces. open() closes its descriptor itself when it fails after the open(2); the temp file is then removed, since its name is new and random, but never on FileExistsError, where the file at that name is not ours.
This was referenced Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups of the drafts 21-29 work (#760), Trello PaFFpE3P (DOCS-ABORT, DOCS-HTML-ROW, DOCS-INDEX-TRUNCATED, F23), plus what three review rounds found in the same command. The F23 strict xfail is gone and its test passes.
What changed
One odd contract no longer ends
fluid docs(DOCS-ABORT). A value of an unexpected type (anidthat is a number,tags: [1, 2], a YAML date or!!set,.nan, an integer too long to print,exposes: 5, a columnsensitivitythat is a list, a lone surrogate) stopped the whole catalog withdocs_failedand a bare Python message, or wrote anindex.jsonstrict parsers reject (NaN). That contract is now listed like the rest: an odd character shown escaped, a set as a sorted list, a non-finite float or an over-long integer as text.index.jsonis strict JSON (allow_nan=False) with json's own key spelling, as before.A failing run no longer leaves a broken catalog (DOCS-INDEX-TRUNCATED).
index.htmlwas opened before it was rendered andindex.jsonstreamed into an open file, so a failed run left one empty or cut off; an id of 242 characters or more made a page name the file system refused, after the index already linked it. Every file is now rendered and encoded first and written to a temp file beside its target. Only when all are written is each moved in, pages before the index. A run that fails while writing keeps the catalog the last run wrote; one that fails while moving files in leaves no dead link.--outis replaced, never written through. The kept mode is set on the temp file's descriptor, never by path, and only for a file of this user with one name.A contract that does not load is a row with its error, not a dead link (DOCS-HTML-ROW).
index.htmllinked it tocontract-contract.html, a page the run never wrote, and its error was only inindex.json.--srclists a product's root contract only, and walks the tree safely (F23). It globbedcontract.fluid.*, so an overlay such ascontract.fluid.prod.yamlgot a page and an index row with no id. It also followed symlinked directories with no loop check (two self-links hung it) and out of--src, and a--srcholding glob characters (my [team] products) gave an empty catalog. It now listscontract.fluid.yaml(.yml,.json) only, withos.walk(followlinks=False), and skips a contract symlink that leads out of--src.--filesstill takes a glob, which follows links.Found on the way:
--outis checked before any contract is read: not a directory, not writable, or behind a part that is no directory. An empty--outsays so, instead of a bare[Errno 2]. A contract that could not be listed is logged with its path.Borrow before build
Bug fixes inside existing code. Patterns taken:
open_filefor the staged write: anO_EXCLtemp file beside the target, thenos.replace. The mode comes from the umask, asopen()gave, not mkstemp's0o600, since the catalog is there to be served.os.walk(followlinks=False)in place of the glob, andjson.dump(allow_nan=False).Validation
-n auto --dist loadscope --ignore=tests/perf): 30502 passed. The 7 failures and 2 collection errors fail identically on clean main:claudeCLI test;tests/fragments+ docs + validate tests, 2074 passed.tests/perf(serial) 38 passed. Green: ruff, black 24.10.0, dead-code gate, product guardrail, import-linter 6/6, mypy --strict hotspots, detect-secrets, license headers. CHANGELOG gate: 45 passed.lstatcheck: the device/inode check at removal already refuses anything else).ripwire --quality-delta(branch diff applied on main):run()stays under the length bar, with the page rendering in_render_pages. Left, with reasons:_render_index_htmlcomplexity 15→17 and length 62→65, and_render_contract_html61→64, all minor: the error row and the escaped values._set_orderand the test helpers. They are called throughsorted(key=...),HTMLParsercallbacks and mockside_effects.prodoverlay, an odd one (id: 12345,.nan, asensitivitylist) and a broken one:index.jsonparses with NaN refused.'<' not supported between instances of 'int' and 'str'.--out ""and--out <a file>are refused before any contract is read.Reviews
--outwalk up its real path), all fixed in the last commits.GATE: PASS.Documentation
docs/cli/docs.md:--srclists root contracts only and does not follow a symlinked directory, a contract that does not load is a row with its error, and the catalog is written whole or not at all. Comes with the forge_docs pages PR of this wave.