fix(validation): close gaps in --global-arg/--input unknown-key check

[stack72]

Follow-ups from the #1312 adversarial review.

Summary

• Prototype-chain bypass: replaced k in shape (walks Object.prototype) with Object.hasOwn(shape, k), so --input constructor=oops (and similar) is now rejected instead of silently dropped.
• ZodEffects unwrapping: schemas built with .refine()/.transform()/.pipe() lack .strict() and .shape on the outer wrapper, so the previous duck-typed cast fell back to strip mode. Added getObjectShape() (using the existing unwrapSchema helper) so the unknown-key check now works against the inner ZodObject. The original schema then runs normal validation, preserving refinements.
• Zod v3 + v4 compatibility: extensions may import npm:zod@3 (uses _def.typeName and _def.shape() as a function) or npm:zod@4 (uses _def.type and _def.shape as a value). The helpers now normalize both. Without this, Zod-v3-based extensions silently dropped unknown keys even with the fix(validation): reject unknown keys in --global-arg and override --input flags #1312 fix in place.
• CLI UX: the model create error no longer shows the confusing leading \: ` prefix from Zod's empty-path \unrecognized_keys` issue. Errors are now explicit and name the valid keys.

Test plan

• Unit tests in `zod_type_coercion_test.ts`: `getObjectShape` works for plain ZodObject, `.refine()`-wrapped, `.optional()`-wrapped, primitive (returns undefined), and Zod v3 `typeName`/`shape()` shapes
• Unit tests in `run_test.ts`: rejects `constructor` key (prototype bypass) and rejects unknown key on `.refine()`-wrapped method arguments
• Unit tests in `create_test.ts`: rejects `constructor` key as global arg and rejects unknown key on `.refine()`-wrapped global arguments
• Manual repro against `/tmp/swamp-repro-issue-244` (which uses `npm:zod@3`): `--global-arg typoKey=oops` and `--global-arg constructor=oops` both now error with a clear message naming the valid keys

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. Slight message phrasing inconsistency across error sites — The three new unknown-key error messages use slightly different prefixes:

   • create.ts: Unknown global argument(s) for type 'X': ...
   • validation_service.ts: Unknown global argument(s): ...
   • method_execution_service.ts: Global arguments validation failed: Unknown argument(s): ...

   None of these are confusing, but the method_execution_service.ts variant uses a different prefix structure. Not blocking — all three are clear and actionable.

2. run.ts uses "Valid inputs are:" while the new messages use "Valid arguments are:" — This inconsistency pre-dates this PR (the run.ts message wasn't changed here), but worth noting for a future cleanup pass.

Verdict

PASS — The primary UX fix (removing the confusing leading : from empty-path Zod errors in create.ts) is a clear improvement. All new unknown-key error messages are explicit, name the offending key(s), and list valid alternatives. No help text, flags, or JSON output shapes were changed.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Repeated unknown-key check pattern — The same shape → filter unknown keys → format error pattern appears in 5 call sites (method_execution_service.ts, validation_service.ts ×2, create.ts, run.ts). A small helper like findUnknownKeys(args, schema): { unknown: string[], valid: string[] } | null that pairs with getObjectShape could reduce the duplication. Each site would only need to handle its own error-reporting mechanism. Not blocking since the current code is correct and each site has different control flow.

2. coerceMethodArgs bracket lookup on shape (zod_type_coercion.ts:122) — shape[key] still uses bracket notation, which walks the prototype chain. If key is "constructor", shape["constructor"] returns the Object constructor (truthy), and getSchemaType(unwrapSchema(...)) is called on it. This is harmless in practice (it gets "" and skips coercion, then the downstream validation rejects the key via Object.hasOwn), but adding an Object.hasOwn(shape, key) guard here would be consistent with the fix applied everywhere else.

Summary

Solid security fix closing three real gaps from the adversarial review: prototype-chain key bypass (k in shape → Object.hasOwn), ZodEffects unwrapping for .refine()/.transform() schemas, and Zod v3/v4 normalization. The getObjectShape helper is well-placed in the domain layer and the test coverage is thorough — covers plain objects, refined schemas, optional wrappers, non-object schemas, and Zod v3 internal structure simulation. No import boundary violations; libswamp-internal code correctly imports from the domain layer. DDD layering is clean.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. method_execution_service.ts:435-445 — hasUnresolved branch skips unknown-key check

   When some global args contain expressions (hasUnresolved = true), the code coerces resolved fields and applies them via setGlobalArgument without running the new Object.hasOwn-based unknown-key check. If a user passes --global-arg constructor=oops --global-arg name=${{inputs.foo}}, the constructor key lands in resolvedGlobalArgs, gets coerced (no-op), and is set on the definition — bypassing the prototype-chain check that the else branch now correctly performs.

   Impact: The stray key has no functional effect (the method won't access it, and the schema doesn't declare it), so this is a UX gap, not a security issue. The user silently loses their typo instead of getting the helpful "Unknown argument(s)" error. This gap existed before this PR (the old .strict() path was also only in the else branch), so it's not a regression — but since this PR is specifically about closing these gaps, it's worth noting.

   Suggested fix: Extract the unknown-key check into a helper and call it in both branches of the hasUnresolved conditional, filtering only against resolved keys.

Low

1. zod_type_coercion.ts:122 — shape[key] without Object.hasOwn in coerceMethodArgs

   coerceMethodArgs looks up field schemas via shape[key] (line 122), which walks the prototype chain. For a key like "constructor", shape["constructor"] resolves to Object.prototype.constructor. This is harmless in practice — the Object function has no _def, so getSchemaType returns "" and no coercion happens — but it's inconsistent with the PR's stated fix of using Object.hasOwn everywhere. Not a security issue since callers always run the Object.hasOwn check afterward.

2. unwrapSchema does not handle ZodPipeline (.pipe())

   The PR description mentions .pipe() alongside .refine() and .transform(), but unwrapSchema only handles the effects type (covering .refine() and .transform()). A ZodPipeline wrapper has type "pipeline" and stores its inner schema as _def.in, not _def.schema. If an extension wraps arguments in .pipe(), getObjectShape returns undefined and the unknown-key check is skipped, falling back to Zod's silent strip mode. This is a pre-existing limitation (.pipe() on argument schemas is unusual) but the PR description's mention of it could mislead readers into thinking it's covered.

Verdict

PASS — The core fixes are correct and well-tested: Object.hasOwn closes the prototype-chain bypass, getObjectShape + unwrapSchema correctly handles ZodEffects wrappers, and the Zod v3/v4 normalization (typeName → type, shape() → shape) is sound. The test coverage is thorough, including mock v3-style schemas. The medium finding is a pre-existing gap in a partial-validation branch, not a regression.