git:20260713.1031a77 to git:20260714.d4fa042

22 added, 4 removed. Audit A to A.

---

name: resource-review

- description: Memory- and resource-safety code review for modern C/C++ (C++17), including Node.js native addons (node-addon-api / Node-API). Use when asked to "review C++/C for memory safety", "find a memory leak", "why does this segfault", "check for use-after-free / double-free / buffer overflow / data race", "resource/handle/fd leak", "review this native addon", "N-API / node-addon-api review", or to check native code with AddressSanitizer/UBSan/TSan/Valgrind. Traces object and resource lifetimes and reports only defects backed by a sanitizer trace, a reproducer, or a fully traced lifetime — never pattern-match guesses.

+ description: Top-level memory- and resource-safety code review for modern C/C++ (C++17), including Node.js native addons (node-addon-api / Node-API). Use when the user asks to "review C++/C for memory safety", "find a memory leak", "why does this segfault", "check for use-after-free / double-free / buffer overflow / data race", "resource/handle/fd leak", "review this native addon", "N-API / node-addon-api review", or to check native code with AddressSanitizer/UBSan/TSan/Valgrind. Reports only defects backed by a sanitizer trace, reproducer, or fully traced lifetime. Do not restart the full workflow for a delegated leaf validation task.

---

# C/C++ Resource & Memory Review

+ ## Leaf-mode guard

+ If the task identifies your role as `leaf-reviewer` or sets

+ `delegation-budget: 0`, read and follow

+ [`references/validation-pass.md`](./references/validation-pass.md), validate only

+ the supplied candidates, return the verdicts to the caller, and stop before the

+ full workflow below.

Identify **provable** memory- and resource-safety defects in modern C/C++ (C++17),

including Node.js native addons built with node-gyp and node-addon-api. Reason about

object lifetimes and resource ownership the way an AddressSanitizer report would — then

report only what you can back with concrete proof. Signal over noise.

This skill finds *defects* (a leak, a use-after-free, a race that actually exists). For

"is this project set up with the right hardening flags, sanitizers, CI, and conventions?"

use the `project-setup` skill, which assesses preventive controls, not exploitable bugs. A

missing hardening flag is not a defect; a heap-buffer-overflow is.

## Scope

**In scope:** C and C++ translation units (`.c/.cc/.cpp/.cxx/.h/.hpp`), especially

Node.js native addons (`Napi::ObjectWrap`, `AsyncWorker`, `ThreadSafeFunction`,

finalizers), the C ABI boundary between C++ and C libraries, and the `binding.gyp`

defines that change runtime behavior.

**Out of scope:** the surrounding JavaScript/TypeScript (defer to

the `web-security-review` skill), and pure build-hardening/convention gaps (defer to

the `project-setup` skill).

### Report vs. research — the prime directive

- **Report on:** only the file, diff, or path the user asked about.

- **Research:** the *entire* module to establish the facts — who allocates, who frees,

which thread touches what, when a destructor or finalizer actually runs, and what the

C library's ownership contract says.

A native defect is almost never visible in one function. Trace the allocation to its

free, the handle to its `close`, the pointer to the object's real lifetime, and the

`napi_value` to the call that created it. **Never report on pattern-match alone.**

## Reporting gate: proof, not probability

Report a finding only when you can **construct concrete proof**. If you cannot describe

how the defect actually manifests at runtime, it is not a finding. Use one of these proof

shapes:

- **Dynamic proof (strongest):** a sanitizer or Valgrind report with a stack trace —

ASan (use-after-free, heap/stack-buffer-overflow, double-free), LSan (leak), UBSan

(signed overflow, misaligned access, invalid cast), TSan (data race), or Valgrind

memcheck/helgrind. Reproduce it when a toolchain is available.

- **Reproducer proof:** a concrete, deterministic sequence of calls (or inputs from JS)

that drives the code to the defect, with the exact lifetime step that goes wrong.

- **Traced-lifetime proof:** the full ownership path in code — the allocation/acquisition

site, every branch to release, and the specific path that double-frees, leaks, frees

early, or reads freed/uninitialized memory — with `file:line` for each step.

Every shape must establish the **trigger** (what sequence or input reaches it), the

**mechanism** (the exact lifetime error), and the **impact** (crash, corruption, leak,

disclosure). Prefer to miss a theoretical issue than to flood the report.

Do **not** assign confidence percentages or 1–10 scores — the complete proof is the

evidence. Every candidate is in exactly one state:

| State | You have… | Action |

| --------------- | ------------------------------------------------------------------------- | ----------------------------------------------- |

| **Proven** | one complete proof shape above, with the lifetime error described concretely | **Report**, with that proof in the finding |

| **Lead** | a suspicious lifetime, ownership ambiguity, or static-analyzer hit with a missing proof element | List under **"Needs verification"** as a question |

| **Theoretical** | a pattern match, style nit, or missing hardening control with no demonstrable error | **Drop** (hardening belongs to the `project-setup` skill) |

Read [`references/proof-and-tooling.md`](./references/proof-and-tooling.md) before

reporting anything — it defines how to obtain each proof and the false-positive

discipline (a static-analyzer warning is a Lead, not a finding; a sanitizer suppression

may be hiding the very bug you are looking for).

## Review workflow

Run these steps in order. Load reference files as each step needs them.

### 1. Scope resolution

- If a path/diff was given, review only that. Otherwise review the working changes,

including untracked files:

```bash

if git rev-parse --verify --quiet origin/HEAD >/dev/null; then

git diff --merge-base origin/HEAD

else

git diff HEAD

git ls-files --others --exclude-standard

```

- Establish the toolchain and shape: language standard and defines from `binding.gyp` /

`CMakeLists.txt`, node-addon-api and `NAPI_VERSION`, whether `NAPI_CPP_EXCEPTIONS` is

set, target OSes/arches, and any vendored C sources. This tells you which classes are

reachable and whether you can build a sanitized reproducer.

### 2. Map the defect surface

For each file, decide which defect classes are even reachable, then load the matching

sections of [`references/defect-classes.md`](./references/defect-classes.md):

| Code under review | Primary classes to check |

| --------------------------------------------------- | --------------------------------------------------------------------- |

| Manual `new`/`delete`, `malloc`/`free`, owning raw pointers | leaks, use-after-free, double/mismatched free |

| OS handles / fds / sockets / library handles | resource/handle/fd leaks, use-after-close |

| Buffers, arrays, pointer arithmetic, `memcpy`/`strcpy` | heap/stack overflow, out-of-bounds read/write |

| Size/length math, casts between int widths/signs | integer overflow/truncation → under-allocation |

| Uninitialized locals/members, partial construction | uninitialized reads |

| Filesystem path checks then use | TOCTOU |

| Threads, `AsyncWorker::Execute`, shared mutable state | data races, lock-ordering, atomics misuse |

| `Napi::ObjectWrap`, `Reference`, `ThreadSafeFunction`, finalizers | Node-API lifetime — see `napi-resource-model.md` |

For any addon code, load

[`references/napi-resource-model.md`](./references/napi-resource-model.md): the

addon-specific hazards a general C++ reviewer misses (call-scoped `napi_value` stored

too long, reference/handle-scope leaks, finalizers that run late or under restricted API

rules, threadsafe-function acquire/release imbalance, exceptions crossing the C ABI).

### 3. Deep lifetime scan

The core pass. **Reason about ownership; do not grep-and-report.** For each allocation or

acquisition, answer: who owns it, is release guaranteed on every path (including

exceptions and early returns), can it be released twice, and can anyone use it after

release? For each buffer, answer: is every index and length bound-checked against the

real allocation size, computed without overflow? Apply the detection signals and safe

patterns in `defect-classes.md`.

### 4. Cross-boundary lifetime analysis

Step back and trace lifetimes *across* function, thread, and language boundaries — where

the real defects hide:

- **C++ ↔ C library:** does the code honor the library's ownership contract (which

allocator frees it, required teardown order, when error accessors are still valid)?

- **Native ↔ JS:** does a JS object outlive the native resource it wraps, or vice versa?

Is a `napi_value`, `Reference`, or `ObjectWrap` pointer used after the object is gone?

- **Thread ↔ thread:** is every field touched from a worker thread and the main thread

synchronized or atomic? Does `Execute()` touch `napi_env`/`napi_value` (forbidden)?

- **Teardown:** what runs during environment/process teardown, and in what order — can a

finalizer, cleanup hook, or destructor touch something already freed?

### 5. Obtain proof

Promote each surviving candidate to Proven or drop it. When a toolchain is available,

build a sanitized target and drive the path (see `proof-and-tooling.md`) — an ASan/UBSan/

TSan/LSan or Valgrind trace is the gold standard. When you cannot build, construct a

complete traced-lifetime proof with `file:line` for every step, or downgrade it to a Lead.

### 6. Adversarial self-verification

- For each surviving candidate, **try to refute it** before it makes the report. For a

- non-trivial finding set, launch one validation subagent per candidate **in parallel**, each

- instructed to disprove the defect using `references/proof-and-tooling.md`:

+ For each surviving candidate, **try to refute it** before it makes the report.

+ For a non-trivial finding set, use at most **two** leaf validation tasks total.

+ Partition or batch the candidates between them; never launch one task per

+ candidate or a second validation round.

+ Prefer the tool-restricted `cpp:reviewer` agent when the host exposes it;

+ otherwise use a general task-local subagent. Start every prompt with

+ `role: leaf-reviewer` and `delegation-budget: 0`, omit workflow skill names, and

+ point it at the resolved path of

+ `<plugin-root>/skills/resource-review/references/validation-pass.md`. When

+ context inheritance is configurable, do not pass the surrounding conversation.

+ Ask each reviewer to disprove its assigned candidates using

+ `references/proof-and-tooling.md`:

- Re-read the lifetime with fresh eyes. Is release *actually* missing, or is there an

RAII guard, `unique_ptr`, tracking set, or cleanup hook that handles it?

- Is the pointer really used after free, or was ownership transferred/nulled first?

- Is the buffer access truly unbounded, or is there an upstream length/`SafeCast` check?

- Is the "race" real, or is the state single-threaded by construction / mutex-protected?

- Is the "leak" first-party, or a one-time library init that a suppression legitimately

covers?

Drop anything without a complete applicable proof after refutation.

### 7. Report

Emit the report in the structure defined by

[`references/report-format.md`](./references/report-format.md): a severity summary table,

then findings grouped by defect class, each with location, proof (sanitizer trace /

reproducer / traced lifetime), a plain-English trigger, impact, and a minimal fix. If

nothing survives, say so explicitly and state what was scanned — "No proven memory/resource

defects identified in <scope>."

### 8. Propose fixes (do not auto-apply)

For each Critical/High finding, propose a concrete, minimal patch (vulnerable → fixed),

preserving surrounding style and names, and prefer the RAII/ownership fix that removes the

class of bug (see the `project-setup` skill's `modern-cpp-conventions.md`) over a one-off

patch. State plainly: **"Review each patch before applying — nothing has been changed."**

Never edit files as part of the review unless the user explicitly asks.

## Severity

| Severity | Impact | Native examples |

| ------------ | ----------------------------------------------- | ------------------------------------------------------------------------------------------------- |

| **Critical** | Memory corruption reachable from untrusted input | Heap-buffer-overflow or use-after-free driven by attacker-influenced JS input; write past a buffer |

| **High** | Corruption/crash under realistic conditions | Use-after-free on a teardown/close race; double-free; unbounded write with a plausible trigger |

| **Medium** | Real defect, bounded or conditional impact | Out-of-bounds *read*; integer truncation feeding a size; leak on a hot path; data race on non-critical state |

| **Low** | Demonstrable but limited | One-time leak only at process exit; missing `close` on a rare error path with small impact |

Rate by demonstrated impact and trigger reachability, not by how alarming the class

sounds. A theoretical overflow with no reachable trigger is not a finding at all.

## Output rules

- Lead with a findings **summary table** (counts by severity).

- **Group by defect class**, not by file.

- Every finding: `file:line`, an evidence snippet, its **proof** (the applicable shape), a

plain-English trigger scenario, impact, and a fix.

- Never auto-apply patches — present them for human review.

- A clean result is a valid result: say what was scanned and that nothing was proven.

## Reference files

Load on demand — keep SKILL.md context lean.

| File | Load during | Covers |

| -------------------------------------------------------------------------- | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------- |

| [`references/defect-classes.md`](./references/defect-classes.md) | steps 2, 3 | Per-class detection signals, safe patterns, CWE/CERT mapping, and which sanitizer proves each: UAF, double/mismatched free, leaks, OOB, uninitialized reads, integer overflow/truncation, TOCTOU, data races |

| [`references/napi-resource-model.md`](./references/napi-resource-model.md) | steps 2, 4 | Node-API / node-addon-api lifetime hazards: call-scoped handles, handle scopes, reference RAII, ObjectWrap finalizer timing and API restrictions, ThreadSafeFunction/AsyncWorker ownership, exceptions across the C ABI, context-aware instance data |

| [`references/proof-and-tooling.md`](./references/proof-and-tooling.md) | steps 5, 6 | The proof hierarchy; running ASan/UBSan/TSan/LSan/Valgrind against a Node addon; what each tool misses; legitimate vs. bug-masking suppressions; the Proven/Lead/Theoretical gate |

| [`references/report-format.md`](./references/report-format.md) | step 7 | Output template and finding card |

## Adapting for your project

Point this skill at `AGENTS.md` and optional `CLAUDE.md` for the module's threading model,

ownership conventions, and known-safe patterns. Record the project's real invariants (for

example "statements are single-threaded by construction", "explicit close orders native

teardown before finalization") in `references/napi-resource-model.md` so the review credits them

instead of re-flagging them, and add any project-specific RAII wrappers to

`references/defect-classes.md` so their release contracts are understood.