mirror of
https://github.com/morluto/rea.git
synced 2026-11-07 05:56:19 -10:00
fix(managed): preserve unknown member ownership across invalid rows
Compare the preceding valid TypeDef coordinate independently for FieldList and MethodList, retaining readable members while leaving affected ownership unknown. Keep the scalar producer and shared owner lookup authoritative across member inspection, metadata inventory and native boundary workflows. Validated with 577 managed/workflow tests, seven malformed/working ordering vectors, and independent dnfile/pefile raw metadata from a pinned public SystemEvents assembly. Four deliberately changed metadata bytes reproduce both decreases across an invalid row. Six CLI calls and ten stdio MCP tool calls before/after preserve all 458 fields and 195 methods and raw signatures/CIL hashes, keep ordinary results identical across three operations, and retain unresolved filter candidates rather than false absence. Alternative #8868 passes the expanded 206 inspector/ownership controls; converge on this generic implementation. The original three unchecked human declarations are preserved. The explicitly authorized readiness-only exception applies after technical CI and review; no human-review attestation is added. Fixes #8861 Full technical CI 42293217445 passes, including all eight shards and required aggregates. Greptile reviews this head at 5/5; Codex cannot complete because the repository review quota is exhausted. The Author readiness-only exception is explicitly authorized; no human-review attestation is added. Assisted-by: Reimu (gensokyo-skills:reimu-incident-triage)
This commit is contained in:
1 parent
fdc4e3908b
commit
a93eb39698
5 files changed
+294
-12
No files matched your search
@@ -739,6 +739,10 @@ refer to the pointer table when present rather than necessarily contiguous
|
||||
definition rows. Invalid or nonmonotonic list coordinates and malformed or
|
||||
ambiguous pointers leave affected owners unknown with partial coverage while
|
||||
preserving readable member and attribute facts.
|
||||
For each list column, ordering checks compare consecutive valid coordinates
|
||||
even across invalid rows. A provable decrease leaves the affected ownership
|
||||
unknown. Retained declaring-type filters count these candidates in
|
||||
`unresolved_filter_rows`; omit that filter to retrieve their known-name rows.
|
||||
The retained inverse indexes are admitted against the existing representation
|
||||
budget before allocation.
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
type ManagedMetadataLayout,
|
||||
} from "./ManagedMetadataLayout.js";
|
||||
import { readManagedPeLayout } from "./ManagedPeReader.js";
|
||||
import { managedMemberListStartsFixture } from "../../tests/fixtures/managedMemberListStarts.js";
|
||||
import { memberListIndexSize } from "./ManagedMemberPointers.js";
|
||||
import { edges } from "./ManagedMemberRows.js";
|
||||
import { parseUserStrings } from "./ManagedUserStrings.js";
|
||||
@@ -2212,6 +2213,127 @@ describe("managed TypeDef list ownership bounds", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("managed TypeDef ordering across invalid coordinates", () => {
|
||||
it.each([
|
||||
{
|
||||
label: "both lists decrease",
|
||||
fields: [2, 0, 1],
|
||||
methods: [2, 0, 1],
|
||||
fieldOwner: null,
|
||||
methodOwner: null,
|
||||
issues: 4,
|
||||
decreases: [
|
||||
["FieldList", 3],
|
||||
["MethodList", 3],
|
||||
],
|
||||
},
|
||||
{
|
||||
label: "out-of-range nonzero coordinates are skipped",
|
||||
fields: [2, 65535, 1],
|
||||
methods: [2, 65535, 1],
|
||||
fieldOwner: null,
|
||||
methodOwner: null,
|
||||
issues: 4,
|
||||
decreases: [
|
||||
["FieldList", 3],
|
||||
["MethodList", 3],
|
||||
],
|
||||
},
|
||||
{
|
||||
label: "known coordinates stay ordered",
|
||||
fields: [1, 0, 1],
|
||||
methods: [1, 0, 1],
|
||||
fieldOwner: "0x02000003",
|
||||
methodOwner: "0x02000003",
|
||||
issues: 2,
|
||||
decreases: [],
|
||||
},
|
||||
{
|
||||
label: "only fields decrease",
|
||||
fields: [2, 0, 1],
|
||||
methods: [1, 1, 1],
|
||||
fieldOwner: null,
|
||||
methodOwner: "0x02000003",
|
||||
issues: 2,
|
||||
decreases: [["FieldList", 3]],
|
||||
},
|
||||
{
|
||||
label: "only methods decrease",
|
||||
fields: [1, 1, 1],
|
||||
methods: [2, 0, 1],
|
||||
fieldOwner: "0x02000003",
|
||||
methodOwner: null,
|
||||
issues: 2,
|
||||
decreases: [["MethodList", 3]],
|
||||
},
|
||||
{
|
||||
label: "multiple invalid rows",
|
||||
fields: [2, 0, 0, 1],
|
||||
methods: [2, 0, 0, 1],
|
||||
fieldOwner: null,
|
||||
methodOwner: null,
|
||||
issues: 6,
|
||||
decreases: [
|
||||
["FieldList", 4],
|
||||
["MethodList", 4],
|
||||
],
|
||||
},
|
||||
{
|
||||
label: "one invalid column does not hide another column or a valid tail",
|
||||
fields: [1, 0, 1],
|
||||
methods: [2, 1, 1],
|
||||
fieldOwner: "0x02000003",
|
||||
methodOwner: "0x02000003",
|
||||
issues: 2,
|
||||
decreases: [["MethodList", 2]],
|
||||
},
|
||||
] as const)(
|
||||
"checks independent ordering across invalid starts: $label",
|
||||
(test) => {
|
||||
const bytes = managedMemberListStartsFixture(test.fields, test.methods);
|
||||
const result = inspectManagedMembersBytes(
|
||||
bytes,
|
||||
managedPeFixtureTarget(bytes),
|
||||
);
|
||||
expect(result.fields).toMatchObject([
|
||||
{
|
||||
token: "0x04000001",
|
||||
declaring_type: test.fieldOwner === null ? null : "Fixture.Program",
|
||||
declaring_type_token: test.fieldOwner,
|
||||
signature: { parse_status: "decoded" },
|
||||
},
|
||||
]);
|
||||
expect(result.methods).toMatchObject([
|
||||
{
|
||||
token: "0x06000001",
|
||||
declaring_type: test.methodOwner === null ? null : "Fixture.Program",
|
||||
declaring_type_token: test.methodOwner,
|
||||
signature: { parse_status: "decoded" },
|
||||
},
|
||||
]);
|
||||
expect(result.types.at(-1)).toMatchObject({
|
||||
field_list: { count: test.fieldOwner === null ? 0 : 1 },
|
||||
method_list: { count: test.methodOwner === null ? 0 : 1 },
|
||||
});
|
||||
expect(result.coverage.state).toBe("partial");
|
||||
expect(result.coverage.issues).toHaveLength(test.issues);
|
||||
expect(
|
||||
result.coverage.issues.filter(({ detail }) =>
|
||||
detail.includes("decreases"),
|
||||
),
|
||||
).toMatchObject(
|
||||
test.decreases.map(([list, row]) => ({
|
||||
code: "invalid-row",
|
||||
scope: `metadata.TypeDef:0x0200000${String(row)}`,
|
||||
detail: expect.stringContaining(
|
||||
`${list} start 1 decreases from preceding start 2`,
|
||||
),
|
||||
})),
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("managed TypeDef list ordering", () => {
|
||||
it("leaves adjacent ownership unknown when valid list starts decrease", () => {
|
||||
const bytes = buildManagedPeFixture({
|
||||
|
||||
@@ -94,29 +94,35 @@ export const readTypeMemberListFacts = (
|
||||
}
|
||||
const nonmonotonicFields = new Set<number>();
|
||||
const nonmonotonicMethods = new Set<number>();
|
||||
for (let index = 0; index + 1 < rows.length; index += 1) {
|
||||
const current = rows[index];
|
||||
const next = rows[index + 1];
|
||||
if (current === undefined || next === undefined) continue;
|
||||
for (const [name, before, after, affected] of [
|
||||
["FieldList", current.field, next.field, nonmonotonicFields],
|
||||
["MethodList", current.method, next.method, nonmonotonicMethods],
|
||||
] as const)
|
||||
const previousKnown = { field: -1, method: -1 };
|
||||
for (const [index, entry] of rows.entries()) {
|
||||
for (const [name, member, affected] of [
|
||||
["FieldList", "field", nonmonotonicFields],
|
||||
["MethodList", "method", nonmonotonicMethods],
|
||||
] as const) {
|
||||
const after = entry[member];
|
||||
// An invalid coordinate cannot establish a range endpoint, but it
|
||||
// cannot hide a decrease between the surrounding valid coordinates.
|
||||
if (after.value === null) continue;
|
||||
const preceding = previousKnown[member];
|
||||
const before = rows[preceding]?.[member];
|
||||
previousKnown[member] = index;
|
||||
if (
|
||||
before !== undefined &&
|
||||
before.value !== null &&
|
||||
after.value !== null &&
|
||||
after.value < before.value
|
||||
) {
|
||||
managedDecodeBudget(layout).reserve(512, after.offset);
|
||||
affected.add(preceding);
|
||||
affected.add(index);
|
||||
affected.add(index + 1);
|
||||
issues.push({
|
||||
code: "invalid-row",
|
||||
scope: `metadata.TypeDef:${metadataToken(2, index + 2)}`,
|
||||
scope: `metadata.TypeDef:${metadataToken(2, index + 1)}`,
|
||||
offset: after.offset,
|
||||
detail: `TypeDef ${name} start ${String(after.raw)} decreases from preceding start ${String(before.raw)}; adjacent member ranges are unknown`,
|
||||
detail: `TypeDef ${name} start ${String(after.raw)} decreases from preceding start ${String(before.raw)}; affected member ranges are unknown`,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
const entries: TypeMemberListEntry[] = rows.map((entry, index) => {
|
||||
const next = rows[index + 1];
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { readFile, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { analysisViewResultSchema } from "../../../src/domain/analysisView/analysisViewResult.js";
|
||||
import { parseEvidence } from "../../../src/domain/evidence.js";
|
||||
import { parseEvidenceBundle } from "../../../src/domain/evidenceBundle.js";
|
||||
import { managedMemberInspectionSchema } from "../../../src/domain/managed/managedArtifact.js";
|
||||
import { managedMemberListStartsFixture } from "../../fixtures/managedMemberListStarts.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
cliTest.for(["decreasing", "ordered"] as const)(
|
||||
"preserves managed ownership and unknown filter rows over CLI/MCP: %s",
|
||||
async (kind, { cli }) => {
|
||||
const root = await createTestTempDirectory("rea-managed-list-order-");
|
||||
const path = join(root, "fixture.dll");
|
||||
const starts = kind === "decreasing" ? [2, 0, 1] : [1, 0, 1];
|
||||
const bytes = managedMemberListStartsFixture(starts, starts);
|
||||
await writeFile(path, bytes);
|
||||
const sha256 = createHash("sha256").update(bytes).digest("hex");
|
||||
const destination = join(root, "members.json");
|
||||
const cliReply = await cli.run({
|
||||
arguments: [
|
||||
"inspect-managed-members",
|
||||
path,
|
||||
"--detail",
|
||||
"summary",
|
||||
"--evidence-bundle",
|
||||
destination,
|
||||
"--json",
|
||||
],
|
||||
});
|
||||
expect(cliReply.exitCode, cliReply.stderr).toBe(0);
|
||||
const bundle = parseEvidenceBundle(
|
||||
JSON.parse(await readFile(destination, "utf8")),
|
||||
);
|
||||
const parent = bundle.records.find(
|
||||
({ operation }) => operation === "inspect_managed_members",
|
||||
);
|
||||
if (parent === undefined)
|
||||
throw new Error("Missing complete member Evidence");
|
||||
expect(parent.subject?.digest.sha256).toBe(sha256);
|
||||
const report = managedMemberInspectionSchema.parse(
|
||||
parent.normalized_result,
|
||||
);
|
||||
expect(report.coverage.state).toBe("partial");
|
||||
expect(
|
||||
report.coverage.issues.filter(({ detail }) =>
|
||||
detail.includes("decreases"),
|
||||
),
|
||||
).toHaveLength(kind === "decreasing" ? 2 : 0);
|
||||
const owner = kind === "decreasing" ? null : "0x02000003";
|
||||
const field = report.fields.find(({ token }) => token === "0x04000001");
|
||||
const method = report.methods.find(({ token }) => token === "0x06000001");
|
||||
if (field?.name == null || method?.name == null)
|
||||
throw new Error("Readable fixture members were lost");
|
||||
for (const row of [field, method]) {
|
||||
expect(row.declaring_type_token).toBe(owner);
|
||||
expect(row.signature.parse_status).toBe("decoded");
|
||||
}
|
||||
|
||||
const call = await cli.sharedMcpCall!();
|
||||
const reply = await call("inspect_managed_members", {
|
||||
path,
|
||||
detail: "summary",
|
||||
});
|
||||
expect(reply.isError, JSON.stringify(reply)).not.toBe(true);
|
||||
const summary = analysisViewResultSchema.parse(
|
||||
parseEvidence(reply.structuredContent).normalized_result,
|
||||
);
|
||||
if (summary.kind !== "summary") throw new Error("Expected member summary");
|
||||
for (const [collection, member] of [
|
||||
["fields", field],
|
||||
["methods", method],
|
||||
] as const) {
|
||||
for (const typed of [false, true]) {
|
||||
const result = await call("inspect_analysis_view", {
|
||||
source: {
|
||||
kind: "retained-evidence",
|
||||
evidence_id: summary.parent_evidence_id,
|
||||
},
|
||||
view: {
|
||||
kind: "managed-members",
|
||||
collection,
|
||||
names: [member.name],
|
||||
fields: ["name", "declaring_type"],
|
||||
offset: 0,
|
||||
limit: 8,
|
||||
...(typed ? { declaring_types: ["Fixture.Program"] } : {}),
|
||||
},
|
||||
});
|
||||
expect(result.isError, JSON.stringify(result)).not.toBe(true);
|
||||
const view = analysisViewResultSchema.parse(
|
||||
parseEvidence(result.structuredContent).normalized_result,
|
||||
);
|
||||
if (view.kind !== "managed-members")
|
||||
throw new Error("Expected retained members");
|
||||
expect(view.page.collection).toBe(collection);
|
||||
expect(view.source_coverage.state).toBe("partial");
|
||||
if (typed && kind === "decreasing") {
|
||||
expect(view.page.rows).toEqual([]);
|
||||
expect(view.page.unresolved_filter_rows).toBe(1);
|
||||
} else {
|
||||
expect(view.page.rows).toMatchObject([
|
||||
{
|
||||
token: member.token,
|
||||
name: member.name,
|
||||
declaring_type_token: owner,
|
||||
},
|
||||
]);
|
||||
expect(view.page.rows[0]).not.toHaveProperty("body");
|
||||
expect(view.page.rows[0]).not.toHaveProperty("signature");
|
||||
}
|
||||
}
|
||||
}
|
||||
expect((await readFile(path)).equals(bytes)).toBe(true);
|
||||
},
|
||||
);
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
import { readManagedMetadataLayout } from "../../src/dotnet/ManagedMetadataLayout.js";
|
||||
import { buildManagedPeFixture } from "../../src/dotnet/ManagedPe.fixture.js";
|
||||
import { readManagedPeLayout } from "../../src/dotnet/ManagedPeReader.js";
|
||||
import { readTypeDefinitionRowFacts } from "../../src/dotnet/ManagedTypeDefinitionRows.js";
|
||||
|
||||
/** Select TypeDef list coordinates while retaining the fixture's readable member rows. */
|
||||
export const managedMemberListStartsFixture = (
|
||||
fields: readonly number[],
|
||||
methods: readonly number[],
|
||||
): Buffer => {
|
||||
if (fields.length === 0 || fields.length !== methods.length)
|
||||
throw new Error("Each fixture TypeDef needs both list coordinates");
|
||||
const bytes = buildManagedPeFixture({
|
||||
additionalEmptyTypes: fields.length - 1,
|
||||
});
|
||||
const pe = readManagedPeLayout(bytes);
|
||||
if (pe.cli === null) throw new Error("Fixture omitted CLI metadata");
|
||||
const { rva, size } = pe.cli.metadata;
|
||||
const layout = readManagedMetadataLayout(
|
||||
bytes,
|
||||
pe.rvaToOffset(rva, size, "cli.metadata"),
|
||||
size,
|
||||
);
|
||||
for (const [index, field] of fields.entries()) {
|
||||
const row = readTypeDefinitionRowFacts(bytes, layout, index + 1);
|
||||
bytes.writeUInt16LE(field, row.fieldListOffset);
|
||||
bytes.writeUInt16LE(methods[index]!, row.methodListOffset);
|
||||
}
|
||||
return bytes;
|
||||
};
|
||||
Reference in new issue
Block a user