Skip to content

chore(noema): reconcile protected main into #2079 - #2198

Merged
seonghobae merged 1 commit into
test/noema-locations-fair-truncationfrom
main
Sep 14, 2026
Merged

seonghobae merged 1 commit into
test/noema-locations-fair-truncationfrom
main

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Ordinary non-force reconciliation of protected main@91be6442906c7b6b4f600272c953699708394327 into canonical Noema finding/probe owner test/noema-locations-fair-truncation. This carries protected-main advances without replacing #2079's three-path semantic delta. No predecessor checks/reviews transfer; #2079 must reacquire exact-head acceptance after merge.


Devin Review

…2194)

Let a repository declare literal path prefixes in
.github/edge-policy-artifact-paths.txt that hold research/data
artifacts (SPSS .sav, serialized model objects, numeric arrays, ...)
not shaped like documentation, and admit binary content there on the
same evidence terms documentation paths already get.

Security property: evaluate_pull_request now threads an optional
base_ref and resolves the declaration only from that ref, never the
pull-request head, so a PR cannot self-authorize admission of its own
binary by adding or widening the declaration in the same diff. A new
test proves the same-PR case is refused. .github/workflows/opencode-
review.yml threads the pull_request_target event's already-available
github.event.pull_request.base.sha with no new permissions.

Suffix decision: a declared-prefix file whose suffix has no
BINARY_DOCUMENT_MAGIC entry (most research formats) is admitted only
on "no diff patch + fetched bytes are not valid UTF-8" evidence, so a
file that decodes as valid UTF-8 is always still content-scanned.
Runtime-named files (_runtime_path_rule) stay rejected inside a
declared prefix exactly as inside docs/ today.

Declaration parsing is bounded (64 entries, 8-segment depth) and
rejects absolute paths, ".." traversal, and globs with a PolicyError
naming the offending entry; a missing declaration file behaves
identically to before this feature existed.

pingora_edge_policy.py stays at 100% branch coverage and 100%
interrogate docstring coverage.

Refs #2193, #2149, #2116.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4a4be452-9904-40c8-9582-cc66b1dff1bc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae merged commit 2b2f348 into test/noema-locations-fair-truncation Sep 14, 2026
85 of 141 checks passed

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +8 to +36

Issue #2193 -- declared research/data artifact paths: a consumer repository may
declare literal path prefixes (``ARTIFACT_PATH_DECLARATION_PATH``) that hold
binary research or data artefacts not shaped like documentation (raw response
workbooks, SPSS ``.sav`` files, serialized model objects, compressed numeric
arrays). That declaration is resolved *only* from the pull request's base ref,
never its head, so a pull request cannot self-authorize admission of its own
binary by adding or widening the declaration in the same diff -- see
``_load_artifact_path_declaration`` and ``evaluate_pull_request``'s ``base_ref``
parameter. The declaration replaces only the path-shape test
(`_is_known_documentation_path`'s equivalent for declared prefixes); it never
substitutes for content evidence, and an active-runtime-named file
(`_runtime_path_rule`) stays rejected inside a declared prefix exactly as inside
``docs/`` today.

Suffix decision: most research-data formats (``.xlsx``, ``.sav``, ``.rds``,
``.npz``, ...) have no entry in ``BINARY_DOCUMENT_MAGIC``, which only knows
``.hwpx``/``.pdf``/``.png``. Rather than grow that registry for every such
format, a file under a declared prefix whose suffix has no magic entry is
admitted on the stricter complement of the UTF-8 decode this module already
performs for every ordinarily-scanned file: no diff patch available, *and* the
fetched bytes fail to decode as UTF-8. That keeps the module's central
guarantee honest -- a file that decodes as valid UTF-8 is never treated as a
binary artifact, since scanning exactly that content is what this module
exists to do -- while still admitting genuinely opaque research binaries
without maintaining an open-ended magic-byte catalog. A suffix that *does*
have a magic entry keeps that entry's existing structural evidence check
(``_is_complete_png``, ``_is_complete_hwpx``, or the raw magic-prefix check for
``.pdf``) even under a declared prefix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 PR description names unrelated scope

The description claims a Noema reconciliation, while the diff changes Pingora policy enforcement. Review and merge records need the actual scope.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 636 to +644
return _is_complete_png(raw)
if suffix == ".hwpx":
return _is_complete_hwpx(raw)
if suffix not in BINARY_DOCUMENT_MAGIC:
try:
raw.decode("utf-8")
except UnicodeDecodeError:
return True
return False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Large declared artifacts still fail

Declared artifacts above MAX_FILE_BYTES fail before the UTF-8 evidence check. Large datasets and model objects remain blocked despite approved prefixes.

(Refers to this code)

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +639 to +643
if suffix not in BINARY_DOCUMENT_MAGIC:
try:
raw.decode("utf-8")
except UnicodeDecodeError:
return True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟥 Declared binaries bypass runtime enforcement

An invalid-UTF-8 file under a declared prefix bypasses CONTENT_RULES when GitHub omits its patch. Runtime candidates without an _runtime_path_rule match can carry prohibited Nginx configuration undetected.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant