Audit: close the credential leak and four other real defects - #34
Merged
Conversation
A launch audit ran eight reviewers, each attacked by an independent skeptic. This commit is the code half of what survived. **A probed endpoint could steal the operator's API key.** Both the media and document probes attached an operator-supplied auth header and followed redirects unconditionally. httpx strips exactly one header across hosts — Authorization — while AuthConfig.header is configurable, and the two commonest real alternatives are not it: Azure OpenAI sends api-key, Azure Functions x-functions-key. So the endpoint under test decided where markproof sent a production credential. The auditor demonstrated it with two local servers; the second logged the key. For a tool whose premise is "point this at your live system with your real token", that is worse than any marking failure it was built to catch. Those cost compliance; this costs a key. Redirects are still followed — an http→https upgrade or a canonical-host redirect is ordinary, and refusing them would make the tool hostile — but every operator-supplied header is dropped the moment the origin changes, where origin is scheme, host and port. A POST body is dropped with it: the prompt is the operator's too. The manifest fetch goes further and refuses to leave the origin at all, because the same-origin check the docs promise was worth nothing while a 302 could move the destination after it. **A correctly signed page was reported as unmarked.** The link parsers matched only quoted attributes and a single-token rel, so `<link rel=c2pa-manifest href=m.c2pa>` — valid HTML5, and what html-minifier's removeAttributeQuotes produces — was invisible, as was `rel="preload c2pa-manifest"`, which RFC 8288 allows. MPF-M-002 is severity fail, so a compliant deployment went red while the finding text asserted there was no link element in bytes containing one. **Three mutations turned text marking into a silent PASS and survived 477 tests.** Nothing asserted end to end that unmarked text produces a non-PASS finding: the detector was tested directly, the engine only ever on a passing fixture. The verdict mapping is now pinned for all six outcomes, with the detector substituted so it runs everywhere rather than only where the optional stack is installed. All three of the auditor's mutations are caught. **A declared obligation with no matching probe vanished.** The README promises that declaring an obligation and supplying nothing to check it with produces a warning rather than a silent skip. That held for a missing watermark config and not for a missing probe: `_unverified_finding` fires inside the per-probe loop, so a rule matching no configured probe produced no finding at all. Declaring synthetic-media-marking against a chat-only target gave a green run and a report that never mentioned the obligation. **Every install command the tool printed was wrong.** Rich reads square brackets as markup, so `pip install 'markproof[synthid]'` reached the user as `pip install 'markproof'` — a command that installs the base package and does not fix what they hit. Finding messages and identifiers go through the same escaping now; table cells are markup too. Also: a configured sign_key that does not resolve stops the run instead of producing an unsigned report with a note nobody reads in CI; the report is written by default, because `markproof init` then `markproof run` produced nothing and the report is the deliverable; and the rulepack digest is joined by a digest over the pattern and label files, since editing one of those flips verdicts while leaving the rulepack byte-identical. The determinism gate now builds reports with every field the CLI fills. `probes` and `data_sha256` were both outside it, which is a blind spot the size of the difference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 1, 2026
Tippel-AI
added a commit
that referenced
this pull request
Sep 1, 2026
…holes (#37) **The README's own demo still ended in a stack trace on a base install.** #22 moved evaluation inside the guarded block and listed the exceptions it knew about. `SynthIdUnavailableError` was not one of them — and the demo config declares `text_marking: synthid`, so the single command the README sends a newcomer to was the case the fix did not reach. Exit 1, which in this tool means "a rule failed". I found it by building a wheel and installing it into a fresh virtualenv rather than by reading, which is the only way that class of defect shows up. The three optional-dependency errors now share a base the CLI catches by type, so the next optional dependency cannot reintroduce it. Verified end to end on a clean install: exit 2, no traceback, and the install command survives intact — which it would not have before the Rich escaping in #34. **The manifest that decides a document verdict was recorded nowhere.** A reader could re-fetch the document, find its digest matching, and still not know which manifest produced the answer. The finding now carries the sidecar digest and how it was found, header or element. **MPF-M-001 passed without saying who signed.** The rulepack allows self-signed chains, so "validly signed" without "by whom" is half an answer. The signer's common name is in the finding now, and a reader decides what it is worth. **A `<link>` in a comment or a script string was taken as the manifest pointer.** Anything that can put text on a page could choose the bytes that decide the verdict. Inert markup is stripped before the scan. **NOTICE and DISCLAIMER both claimed no component is named after a third-party mark.** False: `checks/synthid.py`, `checks/c2pa_verify.py`, the `[synthid]` extra and the `c2pa-verify` check type all are. The narrower parenthetical was true and the sentence around it was not. Both now say what is actually the case — the marks appear where they describe what the code does, which is nominative use, and nowhere as a product name or branding. Renaming them would make the code harder to read and the claim harder to check. **Nothing acknowledged that evidence can contain personal data.** Probes record responses verbatim and reports record the URLs probed. Now stated where the other scope limits are. **The determinism gate asserted nothing about which rules its goldens cover.** It checked that all four verdict values appear, which eleven cases exercising the same two rules would satisfy. Every shipped rule must now appear in some golden. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
The code half of the launch audit. Eight reviewers, each attacked by an
independent skeptic; 56 of 58 findings survived. This closes the ones that are
defects in shipped behaviour.
The one that matters most
A probed endpoint could steal the operator's API key with a 302.
Both probes attached an operator-supplied auth header and followed redirects
unconditionally. httpx strips exactly one header across hosts —
Authorization—while
AuthConfig.headeris configurable, and the two commonest realalternatives are not it:
Authorizationapi-keyx-functions-keyThe auditor ran it: two local servers, the target 302s to the second, and the
second logged
'api-key': 'SECRET-AZURE-KEY-123'. The endpoint under test decidedwhere markproof sent a production credential.
For a tool whose premise is point this at your live system with your real token,
that is worse than any marking failure it was built to catch. Those cost
compliance; this costs a key.
Redirects are still followed — an http→https upgrade is ordinary — but every
operator-supplied header is dropped the moment the origin changes: scheme,
host and port, so a downgrade to http counts. The POST body goes with it; the
prompt is the operator's too. The manifest fetch refuses to leave the origin at
all, because the same-origin check
docs/DISCLAIMER.mdpromises was worth nothingwhile a 302 could move the destination after it.
Four more
A correctly signed page was reported as unmarked. The link parsers matched
only quoted attributes and a single-token
rel, so<link rel=c2pa-manifest href=m.c2pa>— valid HTML5, and exactly what html-minifier'sremoveAttributeQuotesemits — was invisible, as wasrel="preload c2pa-manifest". MPF-M-002 is severity fail, so a compliant deployment went redwhile the finding text asserted there was no link element in bytes that contain
one.
Three mutations turned text marking into a silent PASS and survived 477 tests.
The detector was tested directly; the engine only ever on a passing fixture. The
mapping between them — the part that broke — was unguarded. All six outcomes are
now pinned, with the detector substituted so this runs everywhere rather than only
where the optional stack is installed. All three of the auditor's mutations are
caught.
A declared obligation with no matching probe vanished from the report. The
README promises the opposite. It held for a missing watermark config and not for a
missing probe: declaring
synthetic-media-marking: trueagainst a chat-onlytarget gave a green run and a report that never mentioned the obligation.
Every install command the tool printed was wrong. Rich reads brackets as
markup:
A command that installs the base package and does not fix what the user just hit.
Finding messages and identifiers now go through the same escaping — table cells
are markup too.
Also
sign_keythat does not resolve stops the run, instead of writingan unsigned report with a note nobody reads in CI.
markproof initthenmarkproof runproducednothing, and the report is the deliverable.
--no-reportopts out.disclosure.de-en.yamlflips verdicts while leavingrulepack.sha256byte-identical — a cheaper substitution than the rulepack swap the digest was
added to defend against.
probesanddata_sha256were both outside it.Verification
Every fix is mutation-tested against the defect it closes: removing the
credential-dropping turns 5 tests red, removing the Rich escaping turns 2 red, and
each of the three synthid mutations is caught individually.
512 tests, ruff + mypy strict clean.
Docs and README findings follow in a second PR.
🤖 Generated with Claude Code