Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ This is the key latency decision. We split it:

| Mode | What runs | When | Blocking? |
|---|---|---|---|
| **Sync gate** | Heuristic rules engine (static analysis only — no LLM, no network beyond the tarball we already fetched) | On the tarball request, before bytes are served | Yes — but only on a **cold** package@version. Result is cached by integrity hash, so steady-state is a cache hit (sub-ms). |
| **Sync gate** | Heuristic rules engine (static analysis only — no LLM, no network beyond the tarball we already fetched) | On the tarball request, before bytes are served | Yes — but only on a **cold** package@version. Result is cached by `(name, version, actual integrity)`, so steady-state is a cache hit (sub-ms). |
| **Async enrich** | LLM adapter pass + cross-version diff trend + provenance lookups | Queued after the response is served | No — enrichment updates the stored report and dashboard; never on the request path |

Rationale: the heuristic engine is fast (regex/AST over a tarball that's already in
Expand All @@ -120,9 +120,10 @@ hit, and the verdict is reproducible in CI. Anything slow or non-deterministic
hiccup can never stall an install. The score a client sees inline never depends on
a live LLM.

Caching key = `sha512` integrity from the packument `dist.integrity`. Audits are
immutable per `(package, version, integrity)` — npm can't mutate a published
tarball without changing the hash, so a cached verdict is always valid.
Caching key = `(package name, version, actual sha512 integrity recomputed from
the served bytes)` (ADR-0055). npm can't mutate a published tarball without
changing the hash, while the coordinate dimensions prevent byte-identical
packages from sharing metadata-dependent findings.

### 3.2 Diff-audit

Expand Down Expand Up @@ -832,7 +833,7 @@ audit output.
the target version, runs `remediate`, and walks back a **bounded window**
(newest ≤10 prior versions, via the packument and `cmpSemver`) for the
first version whose own audit is `allow` — short-circuiting on the first
hit and reusing the same cached, integrity-keyed `auditVersion` path every
hit and reusing the same cached, coordinate-and-integrity-keyed `auditVersion` path every
other route uses. A packument fetch failure or a per-version audit failure
is treated as "no last-known-good found," not an error — this is
best-effort advisory output, not a gate. The route is deliberately off the
Expand Down Expand Up @@ -1117,7 +1118,8 @@ throttle protects the expensive read endpoints. Phase 24 closes all four:
`name@version` map lets concurrent uncached public audits for the same
coordinate share one fetch/extract/score pipeline; the entry clears on
settle (success or failure) so a failed run isn't cached. The
integrity-keyed `store` stays the durable cache (invariant #4) — the map
`(name, version, actual integrity)`-keyed `store` stays the durable cache
(invariant #4, ADR-0055) — the map
is transient concurrency dedupe only, scoped to the process.
- **Opt-in rate limiting (`packages/proxy/src/rate-limit.ts`).** A pure
token-bucket `RateLimiter` (`createRateLimiter({ rpm, now })`, injectable
Expand Down Expand Up @@ -1178,7 +1180,7 @@ throttle protects the expensive read endpoints. Phase 24 closes all four:

`limits.ts` and `rate-limit.ts` are pure — no env access, clock injected —
so the caps and the limiter are unit-tested without wall-clock or network
I/O. Scoring, the integrity cache key, and the packument passthrough are
I/O. Scoring, the coordinate-and-integrity cache key, and the packument passthrough are
untouched (invariants #1–#6) — see
[ADR-0037](./docs/adr/0037-resource-robustness.md),
[ADR-0039](./docs/adr/0039-bounded-tarball-extraction.md), and
Expand Down Expand Up @@ -1366,9 +1368,12 @@ Each rule is a pure function `(files, ctx) => Finding[]`. Phase 1 ships four:
3. **`network-egress`** — `http`/`https`/`net`/`dns`, `fetch`, websockets,
`child_process` invoking `curl`/`wget`, hardcoded IPs, suspicious TLDs, and
base64-encoded URLs.
4. **`obfuscation`** — `eval`, `Function(...)` constructor, `atob`/`unescape`,
long base64/hex blobs, `\xNN` string arrays, `charCodeAt` decode loops,
dynamic `require` of decoded strings.
4. **`obfuscation`** — concealed dynamic execution: direct or explicitly-global
JavaScript `eval` (not an arbitrary object method named `eval`), decoded source passed to the `Function`
constructor, and dynamic `require` of decoded/computed strings. Minification,
encoded data assets, data transcoding, character-code operations, and readable
`Function` runtime glue are not findings without a code-execution sink
(ADR-0054).

Phase 8 adds a fifth rule (§3.9, ADR-0021):

Expand Down
13 changes: 8 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ at audit time. Also here: multi-format lockfile parsing (npm/yarn/pnpm) + Cyclon
signed audit attestations (ADR-0032), `lintPolicy` (ADR-0033).

**Proxy (`@git-agentic/sentinel-proxy`)** — sync inline gate over bytes in memory, cached by
`dist.integrity`; transparent packument passthrough rewriting only `dist.tarball`.
`(name, version, actual integrity)`; transparent packument passthrough rewriting only `dist.tarball`.
Private-namespace packages are served only from the private store, fail-closed
(ADR-0010/0015). `POST /-/audit-tree` whole-lockfile gate with dedupe + 413 cap
(ADR-0020/0037); `GET /-/explain` walk-back (ADR-0031); `POST /-/policy/preview`
Expand Down Expand Up @@ -167,10 +167,13 @@ enforcement is tested with benign probe packages.
`NoopLlmAdapter`; the engine is fully offline.
3. **The inline gate is sync + cheap; everything slow is async.** The proxy audits
on the tarball request (static analysis over bytes already in memory) and caches
by `dist.integrity`. Never put a network call or an LLM call on the request path.
4. **Cache key = integrity hash.** A published tarball is immutable, so
`(name, version, integrity)` is a safe immutable key. Don't key caches on
version alone.
by `(name, version, actual integrity)`. Never put a network call or an LLM call
on the request path.
4. **Cache key = coordinate + integrity hash.** Complete reports contain
coordinate-dependent findings, so `(name, version, actual integrity)` is the
safe immutable key. Integrity remains mandatory so changed bytes always miss;
name/version prevent byte-identical coordinates from sharing findings
(ADR-0055). Don't key caches on version or integrity alone.
5. **The proxy is transparent.** For packuments we pass the upstream document
through and rewrite *only* `dist.tarball`. Don't synthesize or strip fields on
the npm path — it breaks resolution (dependencies, peer deps, etc.).
Expand Down
118 changes: 118 additions & 0 deletions docs/adr/0054-obfuscation-execution-correlation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# ADR-0054: Obfuscation findings require a dynamic code-execution sink

**Status:** Accepted
**Date:** 2026-08-06
**Amends:** the Phase 1 `obfuscation` rule semantics in `ARCHITECTURE.md` §4.1

## Context

The original `obfuscation` rule treated each of these as independent evidence:
`eval`, the `Function` constructor, base64 decoding, `unescape`, long encoded
strings, `\xNN` runs, and character-code operations. That conflated three
different facts:

1. data is encoded or transcoded;
2. code is generated dynamically; and
3. concealed code is executed.

Only the third is evidence of the behavior this rule is named for. npm packages
legitimately ship encoded WASM/data assets, minified bundles, compatibility
transcoders, and runtime glue. Those shapes are not evidence of obfuscation on
their own.

The overly broad matcher also used `\beval\s*\(`, which matched any property
method named `eval`. In `@emnapi/core@1.9.2`, `g.eval(v8Script.value)` implements
a WebAssembly/N-API runtime operation; it is not JavaScript's direct evaluator.
The same package contains readable `new Function(...)` glue for constructing a
named wrapper. Sentinel emitted repeated high findings for both constructs
across the package's generated bundle formats. A package that bundled emnapi,
such as `@tailwindcss/oxide-wasm32-wasi`, inherited those findings for its
vendored copies.

ADR-0053 bounded repeated penalties but intentionally did not change detection
semantics. A cold calibration at its `a65ab42` baseline still had 70 production-
tree blocks involving `obfuscation`. Further arithmetic suppression would hide
the attribution error rather than correct it.

## Decision

The `obfuscation` rule reports evidence only when the source contains a dynamic
code-loading or execution sink:

- **JavaScript `eval(...)`** remains `high`, both as a direct identifier call and
through an explicit standard global object (`globalThis`, Node's `global`, or
browser `window`/`self`). An arbitrary method/property named `eval` (for
example `runtime.eval(...)`) is excluded because its semantics are defined by
that object, not the language evaluator.
- **A computed `require(...)` target** produced through the existing decoded or
opaque-call shapes remains `high`.
- **The `Function` constructor** is `high` only when its source is decoded
inline or comes from a locally assigned `atob(...)` / base64
`Buffer.from(...)` result. A readable constructor body or runtime wrapper is
dynamic code generation, but not concealed code.

These patterns are not `obfuscation` findings without such a sink:

- minification or generated bundles;
- correctly declared WASM/native/binary assets;
- a long base64/hex string;
- base64 decoding, `atob`, `unescape`, or character-code conversion used as
data transcoding;
- `\xNN` data runs; or
- readable `Function`-constructor runtime glue.

This is a rule-semantics change, not a scoring-policy change. The rule remains a
pure deterministic `(AuditInput) => Finding[]` function and still constructs
every finding through `mkFinding()`. `DEFAULT_POLICY`, thresholds, severity
weights, the per-rule cap, and the pre-registered calibration criteria are
unchanged.

## Why this does not trade attack coverage for a target number

The removed primitives establish encoding or code generation, not concealed
code execution. Sentinel continues to flag the execution paths: direct or
explicitly-global eval, decoded `Function` source, and computed decoded
`require`. The committed
synthetic malicious release decodes a base64 stage and passes it to direct
`eval`; it therefore retains its high obfuscation finding and remains blocked at
score 0. Native packaged-payload materialization and execution also remains
covered independently by the dataflow-correlated `native-payload-loader` rule
(ADR-0049), including correctly declared payloads and content mismatches.

If future evidence shows a dangerous shape that does not reach one of these
sinks, add a correlation that describes that behavior. Do not restore a
standalone "encoded/minified data is obfuscation" penalty.

## Consequences

- Cold direct audits after this change produce no `obfuscation` findings for
`@emnapi/core@1.9.2` (score 84, `allow`) or
`@tailwindcss/oxide-wasm32-wasi@4.1.16` (score 80, `allow`). Other independent
rule findings account for the remaining deductions.
- Finding messages and remediation now describe concealed dynamic execution,
not "minified beyond normal" source.
- The local decoded-variable correlation is intentionally narrow and
deterministic. It recognizes direct assignments from `atob` and base64
`Buffer.from`; it is not represented as full JavaScript dataflow analysis.
- The complete four-tree calibration must be rerun cold in the consuming repo;
these package probes validate the named false-positive class but do not
substitute for that registered measurement.

## Rejected alternatives

### Lower weights or move thresholds

Rejected. Those are policy choices and would make the number smaller without
fixing the false attribution.

### Lower the per-rule cap again

Rejected. ADR-0053 already bounds repetition while retaining breadth of
evidence. Tightening it for this case would suppress every rule instance rather
than distinguish data assets from concealed code execution.

### Package or path allowlists

Rejected. The decision is based on behavior and applies equally to first-party,
vendored, scoped, and unscoped packages. No package name, publisher, filename,
or known hash changes the result.
97 changes: 97 additions & 0 deletions docs/adr/0055-coordinate-bound-audit-cache.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# ADR-0055: Bind cached audit reports to package coordinate and integrity

**Status:** Accepted
**Date:** 2026-08-06
**Extends:** ADR-0004 (integrity-hash cache key), ADR-0012 (policy-bound verdicts)

## Context

ADR-0004 correctly requires the actual tarball integrity in every cache key so
changed bytes can never reuse a stale verdict. It also states that the logical
identity is `(name, version, integrity)`. The implementation did not match that
logical identity: `AuditStore` stored and looked up reports in a map keyed only
by `integrity`.

An `AuditReport` is not a pure function of tarball bytes. Several deterministic
rules also consume the requested coordinate or its packument context:

- `typosquat` consumes the package name;
- known advisories and vulnerabilities consume name and version;
- release anomaly consumes version history and release metadata;
- report metadata, remediation, and last-known-good output repeat the requested
name and version.

Two package coordinates may legitimately serve byte-identical tarballs. With an
integrity-only report map, whichever coordinate was audited first owned the
cache entry. A later `GET /-/explain/:name/:version` returned that first
coordinate's metadata and finding set. This was reproduced hermetically with
`express@1.0.0` and the lookalike `expres@1.0.0` sharing one fixture tarball:
the second response identified itself as `express` and omitted its `typosquat`
finding.

## Decision

`AuditStore` keys every cached report by the exact tuple:

```
(normalized package name, version, actual tarball integrity)
```

The implementation uses an unambiguous NUL-delimited internal key. Persistent
schema-3 rows are re-indexed from their stored `name`, `version`, and report
integrity on load. Every report-serving cache lookup supplies all three
dimensions; there is no integrity-only serving fallback.

The actual bytes hash remains mandatory and load-bearing. A re-publish or
tampered mirror that changes bytes changes the integrity dimension and therefore
misses the cache exactly as ADR-0004 requires. Adding coordinate dimensions does
not weaken content addressing; it prevents byte-identical coordinates from
sharing coordinate-dependent findings.

The proxy runs one active enterprise policy per `AuditStore`, and persisted
reports whose `policy.hash` differs from that active policy are rejected during
load (ADR-0012). This ADR does not add the policy hash to the in-memory tuple
because the active store already provides that isolation.

Approval, approval-request, and violation writes continue to record their
serve-time overlay by integrity. Approval requests and violations already submit
name/version and use the exact tuple. For backward compatibility, the older
approval endpoint may omit name/version; that integrity-only lookup succeeds only
when exactly one cached coordinate has those bytes. Shared-byte ambiguity fails
closed and requires the caller to submit name/version, preventing an arbitrary
coordinate's report from being borrowed.

## Consequences

- `/-/explain`, `/-/audit`, `/-/manifest`, tarball serving, and `/-/audit-tree`
all receive the report for the requested coordinate even when another package
uses identical bytes.
- `AuditStore.stats().total` and `recent()` count coordinate-bound audit records,
not unique tarball blobs. This matches their report/history semantics.
- Cache hits remain sub-millisecond map lookups and still require the actual
integrity computed from fetched bytes.
- Existing persisted schema-3 rows remain readable; they are indexed under the
corrected tuple at startup. Schema-1/2 rejection is unchanged.
- Engine/scorer-version invalidation is separate from coordinate identity. A
calibration after detection changes must still use a cold store until an
explicit engine-version cache dimension or migration policy is designed.

## Rejected alternatives

### Keep integrity-only reports and rewrite `meta` on response

Rejected. Name/version are inputs to findings, not presentation-only fields.
Rewriting metadata would still reuse incorrect typosquat, advisory,
vulnerability, and release-anomaly results.

### Cache byte-only extraction and rerun coordinate rules on every hit

Potentially valid as a deeper cache split, but rejected for this fix. The current
public cache stores complete `AuditReport` objects, and separating byte-derived
observations from coordinate-derived rules would require a new persisted schema.
The tuple key fixes correctness without changing the audit pipeline.

### Key only by `(name, version)`

Rejected for the original ADR-0004 reason: changed or tampered bytes under the
same version label must never reuse a verdict.
4 changes: 3 additions & 1 deletion docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ not yet built · `Superseded` / `Deprecated` — replaced; see the linked succes
| [0001](./0001-auditing-proxy-wedge.md) | Auditing proxy, not an npm replacement | Insert a transparent proxy; attach signal, don't own packages |
| [0002](./0002-deterministic-scoring-llm-enrichment.md) | Deterministic scoring; LLM enrichment only | Rules set the score; the LLM may only add context, never the verdict |
| [0003](./0003-sync-gate-async-enrich.md) | Sync gate / async enrich split | Cheap deterministic gate inline; everything slow or networked runs async |
| [0004](./0004-integrity-hash-cache-key.md) | Integrity-hash cache key | Key verdicts on the tarball SRI hash — content-addressed, never stale |
| [0004](./0004-integrity-hash-cache-key.md) | Integrity-hash cache key | Key verdicts on the tarball SRI hash — content-addressed, never stale; logical coordinate binding implemented by ADR-0055 |
| [0005](./0005-transparent-packument-passthrough.md) | Transparent packument pass-through | Forward the upstream doc; rewrite only `dist.tarball` |
| [0006](./0006-stack-node-typescript-workspaces.md) | Stack: Node + TS + npm workspaces | Live in the runtime we audit; one shared `AuditReport` contract |
| [0007](./0007-client-integration-registry-redirection.md) | Integrate via registry redirection | Point `registry` at the proxy — covers all PMs + transitive deps |
Expand Down Expand Up @@ -152,6 +152,8 @@ first shipped slice; Phases 31 and 32 complete claiming and retraction.
| ADR | Title | Decision in one line |
|-----|-------|----------------------|
| [0053](./0053-per-rule-score-cap.md) | Per-rule score cap | `scoring.perRuleCapMultiplier` (policy data, default 3) bounds a rule's total penalty at that multiple of its own worst-instance weight, so N file-level findings from one rule can't alone drive the score to zero; monotonic, waiver-compatible (ADR-0014), optional for backward compat with already-signed policies |
| [0054](./0054-obfuscation-execution-correlation.md) | Obfuscation requires an execution sink | Encoded/minified data and readable runtime glue are not obfuscation by themselves; retain high findings for direct `eval`, decoded `Function` source, and computed decoded `require` |
| [0055](./0055-coordinate-bound-audit-cache.md) | Coordinate-bound audit cache | Bind complete reports to `(name, version, actual integrity)` so byte-identical package coordinates cannot share metadata-dependent findings |

## Conventions

Expand Down
Loading
Loading