frozenCopy hands a non-plain-prototype subtree of a guarded field to a neighbour live and unfrozen, so LLP 0423's copy invariant is open on that route

Open
  • Is something wrong?
#2,116 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
52/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
javascript
Domain
backend

Research direction

Read src/core/runtime/activation.js and the ownership tests in test/core/command-context-plugin-ownership.test.js; trace frozenCopy through the real ctx.verbs.get() path. Add coverage for class-instance inputSchema.properties and root schemas, checking that writes do not reach the registrant and a second read remains unchanged. Run the ownership tests, including the existing 20000-deep case.

Written by the indexing model from the issue text.

Description

Source: #2111 at 2be006143c2e25d8c4ed2ca6ba271b2c678dacd1 (pre-existing; identical on the PR's base 32d00dea, so not a regression from that PR)
Location: src/core/runtime/activation.js, frozenCopy's shell (and the same test on the base's recursive form)
Severity: minor

Evidence

LLP 0423 #consequences records issue #1985 as closed by making narrowView
answer for inputSchema and aliases with "a frozen structural copy, taken on
the first read of the field and kept for the life of the view, so neither the
argv codec's rules nor the advertised tool schema is a neighbour's to edit."

frozenCopy copies plain objects, null-prototype objects and arrays. Anything
else it returns by reference, unfrozen:

const proto = Object.getPrototypeOf(entry)
if (proto !== Object.prototype && proto !== null) return entry

So a registrant that declares any part of its schema as a non-plain object
hands every neighbour the live object. Measured through the real
ctx.verbs.get() path, one plugin registering and a second reading:

class Props { constructor() { this.real = { type: 'string', description: 'original' } } }
const schema = { type: 'object', properties: new Props(), required: [] }
// A registers it; B reads it
const copy = B.verbs.get('owner np2').inputSchema
copy.properties.real.type = 'OVERWRITTEN'
copy.properties.injected = { type: 'boolean', default: true }

rootFrozen                     true
nestedIsLiveRegistrantObject   true      <- copy.properties === schema.properties
nestedFrozen                   false
write                          ok
registrant's declaration now   {"real":{"type":"OVERWRITTEN","description":"original"},"injected":{"type":"boolean","default":true}}
a second read sees it          {"real":{"type":"OVERWRITTEN","description":"original"},"injected":{"type":"boolean","default":true}}

Identical on 32d00dea and on #2111's head 2be00614, so #2111 neither caused
nor widened it.

The same hole reaches the root: an inputSchema that is itself a class
instance is handed back live and unfrozen, because validateVerb
(src/core/registry/verbs.js:315) checks only
!inputSchema || typeof inputSchema !== 'object'.

Over 3000 seeded graphs driven through the shipped walk, 2897 nodes in the
returned copies were live source objects passed through this branch. No plain
object or array is ever reached by reference (measured: 0), so the hole is
exactly the non-plain-prototype branch.

Expected behavior

A neighbour reading a guarded field cannot write into the registrant's own
declaration, whatever prototype the registrant gave the objects in it, which is
the property LLP 0423 #consequences records as closing #1985.

Why deferral is safe

The blast radius is the registrant's own declaration: the live objects reached
are the ones that registrant allocated, so there is no cross-plugin reach
beyond the two parties and nothing a registrant could not have declared
directly. Core's own verbs declare plain object literals, so the #1985
headline case (hyp query sql's include-local-only default, and pushing onto
required) is not reachable this way; a grep of src/ and
hypaware-core/plugins-workspace/ finds no in-tree registration with a
non-plain-object inputSchema. The pass-through is also deliberate as far as
it goes: freezing a registrant's Date, Map or class instance would mutate
the registrant's own object rather than copy it, which is the trade
frozenCopy declined, so closing this means deciding what a non-plain value in
a guarded field should become (dropped, replaced by a frozen plain projection,
or refused at registration) rather than flipping a branch.

Acceptance condition

A test in test/core/command-context-plugin-ownership.test.js where plugin A
registers a verb whose inputSchema.properties is a class instance and plugin
B reads the narrowed view: B's write must not reach A's declaration, and a
second read must not observe it. The root-level variant (the whole
inputSchema a class instance) must hold too. Existing ownership tests and the
20000-deep test added by #2111 must still pass unchanged.

Evidenced behavioral defect; eligible for automatic repair.

Dominant language
JavaScript
Stars
11
Forks
0
Avg merge
2h 49m
Merged PRs (30d)
302

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from hyparam/hypaware

All issues in hyparam/hypaware

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.