Skip to content

Commit

Permalink
fix(orchestration): harden exported patterns (#10470)
Browse files Browse the repository at this point in the history
closes: #XXXX
refs: #XXXX

## Description

orchestration/src/typeGuards was defining and exporting some patterns as just unhardened object literals. This is locally wrong, since these are not valid patterns (or even passables) until hardened. The only reason these errors were uncaught must be because they were then used in a context which happened to harden them before they were used in a context where they were validated as patterns or passables.

Started as a drive-by of #10456 , where I import some of these patterns for usage elsewhere.

### Security Considerations

defensive hardening is good

### Scaling Considerations

none

### Documentation Considerations

one less thing we would need to explain

### Testing Considerations

In theory, a test could have revealed the bug that these were exported unhardened. But IMO, such tests would only be too much noise. We should just fix these. But this PR only fixes these in this one file. I don't know how many others there are.

### Upgrade Considerations

In theory, it is possible that some importing code somewhere accidentally replies on these being unhardened, perhaps because it mutates one in place before using it, corrupting use by all other importers. If so, this PR will break such uses, which is good, because the silent corruption is a worse surprise than a new noisy error.
  • Loading branch information
erights authored Nov 15, 2024
1 parent 5a127d4 commit 47bebb8
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 111 deletions.
22 changes: 10 additions & 12 deletions packages/ERTP/src/typeGuards.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ const AmountValueShape = M.or(
CopyBagValueShape,
);

export const AmountShape = harden({
brand: BrandShape,
value: AmountValueShape,
});
export const AmountShape = { brand: BrandShape, value: AmountValueShape };
harden(AmountShape);

/**
* To be used to guard an amount pattern argument, i.e., an argument which is a
Expand All @@ -94,10 +92,8 @@ export const AmountShape = harden({
export const AmountPatternShape = M.pattern();

/** @type {TypedPattern<Ratio>} */
export const RatioShape = harden({
numerator: AmountShape,
denominator: AmountShape,
});
export const RatioShape = { numerator: AmountShape, denominator: AmountShape };
harden(RatioShape);

/**
* Returns true if value is a Nat bigint.
Expand Down Expand Up @@ -158,13 +154,14 @@ export const DisplayInfoShape = M.splitRecord(
},
);

export const IssuerKitShape = harden({
export const IssuerKitShape = {
brand: BrandShape,
mint: MintShape,
mintRecoveryPurse: PurseShape,
issuer: IssuerShape,
displayInfo: DisplayInfoShape,
});
};
harden(IssuerKitShape);

// //////////////////////// Interfaces /////////////////////////////////////////

Expand Down Expand Up @@ -235,10 +232,11 @@ export const makeIssuerInterfaces = (
receive: getInterfaceGuardPayload(PurseI).methodGuards.deposit,
});

const PurseIKit = harden({
const PurseIKit = {
purse: PurseI,
depositFacet: DepositFacetI,
});
};
harden(PurseIKit);

return harden({
IssuerI,
Expand Down
5 changes: 3 additions & 2 deletions packages/SwingSet/src/typeGuards.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ export const ManagerType = M.or(

const Bundle = M.splitRecord({ moduleType: M.string() });

const SwingsetConfigOptions = harden({
const SwingsetConfigOptions = {
creationOptions: M.splitRecord({}, { critical: M.boolean() }),
parameters: M.recordOf(M.string(), M.any()),
});
};
harden(SwingsetConfigOptions);

const SwingSetConfigProperties = M.or(
M.splitRecord({ sourceSpec: M.string() }, SwingsetConfigOptions),
Expand Down
143 changes: 88 additions & 55 deletions packages/governance/src/typeGuards.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,31 @@ export const ElectionTypeShape = M.or(
'offer_filter',
);

export const ClosingRuleShape = harden({
export const ClosingRuleShape = {
timer: M.eref(TimerShape),
deadline: TimestampShape,
});
};
harden(ClosingRuleShape);

// all the strings that will be in the filter after passing
export const YesOfferFilterPositionShape = harden({
export const YesOfferFilterPositionShape = {
strings: M.arrayOf(M.string()),
});
export const NoOfferFilterPositionShape = harden({
dontUpdate: M.arrayOf(M.string()),
});
export const OfferFilterPositionsShape = harden([
};
harden(YesOfferFilterPositionShape);

export const NoOfferFilterPositionShape = { dontUpdate: M.arrayOf(M.string()) };
harden(NoOfferFilterPositionShape);

export const OfferFilterPositionsShape = [
YesOfferFilterPositionShape,
NoOfferFilterPositionShape,
]);
export const OfferFilterIssueShape = harden({
strings: M.arrayOf(M.string()),
});
export const OfferFilterQuestionSpecShape = harden({
];
harden(OfferFilterPositionsShape);

export const OfferFilterIssueShape = { strings: M.arrayOf(M.string()) };
harden(OfferFilterIssueShape);

export const OfferFilterQuestionSpecShape = {
method: ChoiceMethodShape,
issue: OfferFilterIssueShape,
positions: OfferFilterPositionsShape,
Expand All @@ -46,34 +51,41 @@ export const OfferFilterQuestionSpecShape = harden({
closingRule: ClosingRuleShape,
quorumRule: QuorumRuleShape,
tieOutcome: NoOfferFilterPositionShape,
});
export const OfferFilterQuestionDetailsShape = harden({
};
harden(OfferFilterQuestionSpecShape);

export const OfferFilterQuestionDetailsShape = {
...OfferFilterQuestionSpecShape,
questionHandle: makeHandleShape('Question'),
counterInstance: InstanceHandleShape,
});
};
harden(OfferFilterQuestionDetailsShape);

// keys are parameter names, values are proposed values
export const ParamChangesSpecShape = M.recordOf(M.string(), M.any());
export const YesParamChangesPositionShape = ParamChangesSpecShape;
export const NoParamChangesPositionShape = harden({
noChange: M.arrayOf(M.string()),
});
export const ParamChangesPositionsShape = harden([
export const NoParamChangesPositionShape = { noChange: M.arrayOf(M.string()) };
harden(NoParamChangesPositionShape);

export const ParamChangesPositionsShape = [
YesParamChangesPositionShape,
NoParamChangesPositionShape,
]);
export const ParamPathShape = harden({
key: M.any(),
});
export const ParamChangesIssueShape = harden({
];
harden(ParamChangesPositionsShape);

export const ParamPathShape = { key: M.any() };
harden(ParamPathShape);

export const ParamChangesIssueShape = {
spec: {
paramPath: ParamPathShape,
changes: ParamChangesSpecShape,
},
contract: InstanceHandleShape,
});
export const ParamChangesQuestionSpecShape = harden({
};
harden(ParamChangesIssueShape);

export const ParamChangesQuestionSpecShape = {
method: 'unranked',
issue: ParamChangesIssueShape,
positions: ParamChangesPositionsShape,
Expand All @@ -83,27 +95,33 @@ export const ParamChangesQuestionSpecShape = harden({
closingRule: ClosingRuleShape,
quorumRule: 'majority',
tieOutcome: NoParamChangesPositionShape,
});
};
harden(ParamChangesQuestionSpecShape);

export const ParamChangesQuestionDetailsShape = harden({
export const ParamChangesQuestionDetailsShape = {
...ParamChangesQuestionSpecShape,
questionHandle: makeHandleShape('Question'),
counterInstance: InstanceHandleShape,
});
};
harden(ParamChangesQuestionDetailsShape);

const ApiInvocationSpecShape = harden({
const ApiInvocationSpecShape = {
apiMethodName: M.string(),
methodArgs: M.arrayOf(M.any()),
});
};
harden(ApiInvocationSpecShape);

export const YesApiInvocationPositionShape = ApiInvocationSpecShape;
export const NoApiInvocationPositionShape = harden({
dontInvoke: M.string(),
});
export const ApiInvocationPositionsShape = harden([
export const NoApiInvocationPositionShape = { dontInvoke: M.string() };
harden(NoApiInvocationPositionShape);

export const ApiInvocationPositionsShape = [
YesApiInvocationPositionShape,
NoApiInvocationPositionShape,
]);
export const ApiInvocationQuestionSpecShape = harden({
];
harden(ApiInvocationPositionsShape);

export const ApiInvocationQuestionSpecShape = {
method: 'unranked',
issue: ApiInvocationSpecShape,
positions: ApiInvocationPositionsShape,
Expand All @@ -113,39 +131,53 @@ export const ApiInvocationQuestionSpecShape = harden({
closingRule: ClosingRuleShape,
quorumRule: QuorumRuleShape,
tieOutcome: NoApiInvocationPositionShape,
});
export const ApiInvocationQuestionDetailsShape = harden({
};
harden(ApiInvocationQuestionSpecShape);

export const ApiInvocationQuestionDetailsShape = {
...ApiInvocationQuestionSpecShape,
questionHandle: makeHandleShape('Question'),
counterInstance: InstanceHandleShape,
});
};
harden(ApiInvocationQuestionDetailsShape);

const SimpleSpecShape = harden({
text: M.string(),
});
export const YesSimplePositionShape = harden({ text: M.string() });
export const NoSimplePositionShape = harden({ text: M.string() });
export const SimplePositionsShape = harden([
const SimpleSpecShape = { text: M.string() };
harden(SimpleSpecShape);

export const YesSimplePositionShape = { text: M.string() };
harden(YesSimplePositionShape);

export const NoSimplePositionShape = { text: M.string() };
harden(NoSimplePositionShape);

export const SimplePositionsShape = [
YesSimplePositionShape,
NoSimplePositionShape,
]);
];
harden(SimplePositionsShape);

export const SimpleIssueShape = SimpleSpecShape;
export const SimpleQuestionSpecShape = harden({
harden(SimpleIssueShape);

export const SimpleQuestionSpecShape = {
method: ChoiceMethodShape,
issue: SimpleIssueShape,
positions: M.arrayOf(harden({ text: M.string() })),
positions: M.arrayOf({ text: M.string() }),
electionType: M.or('election', 'survey'),
maxChoices: M.gte(1),
maxWinners: M.gte(1),
closingRule: ClosingRuleShape,
quorumRule: QuorumRuleShape,
tieOutcome: NoSimplePositionShape,
});
export const SimpleQuestionDetailsShape = harden({
};
harden(SimpleQuestionSpecShape);

export const SimpleQuestionDetailsShape = {
...SimpleQuestionSpecShape,
questionHandle: makeHandleShape('Question'),
counterInstance: InstanceHandleShape,
});
};
harden(SimpleQuestionDetailsShape);

export const QuestionSpecShape = M.or(
ApiInvocationQuestionSpecShape,
Expand Down Expand Up @@ -199,11 +231,12 @@ export const ElectorateCreatorI = M.interface('Committee AdminFacet', {
getPublicFacet: M.call().returns(ElectoratePublicShape),
});

export const QuestionStatsShape = harden({
export const QuestionStatsShape = {
spoiled: M.nat(),
votes: M.nat(),
results: M.arrayOf({ position: PositionShape, total: M.nat() }),
});
};
harden(QuestionStatsShape);

export const QuestionI = M.interface('Question', {
getVoteCounter: M.call().returns(InstanceHandleShape),
Expand Down
29 changes: 17 additions & 12 deletions packages/orchestration/src/typeGuards.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import { M } from '@endo/patterns';

/**
* @import {TypedPattern} from '@agoric/internal';
* @import {ChainAddress, CosmosAssetInfo, Chain, ChainInfo, CosmosChainInfo, DenomAmount, DenomDetail, DenomInfo, AmountArg, CosmosValidatorAddress} from './types.js';
* @import {ChainAddress, CosmosAssetInfo, Chain, ChainInfo, CosmosChainInfo, DenomAmount, DenomInfo, AmountArg, CosmosValidatorAddress} from './types.js';
* @import {Any as Proto3Msg} from '@agoric/cosmic-proto/google/protobuf/any.js';
* @import {Delegation} from '@agoric/cosmic-proto/cosmos/staking/v1beta1/staking.js';
* @import {TxBody} from '@agoric/cosmic-proto/cosmos/tx/v1beta1/tx.js';
* @import {TypedJson} from '@agoric/cosmic-proto';
*/
Expand All @@ -31,12 +30,11 @@ export const ChainAddressShape = {
encoding: M.string(),
value: M.string(),
};
harden(ChainAddressShape);

/** @type {TypedPattern<Proto3Msg>} */
export const Proto3Shape = {
typeUrl: M.string(),
value: M.string(),
};
export const Proto3Shape = { typeUrl: M.string(), value: M.string() };
harden(ChainAddressShape);

/** @internal */
export const IBCTransferOptionsShape = M.splitRecord(
Expand All @@ -53,6 +51,7 @@ export const IBCTransferOptionsShape = M.splitRecord(

/** @internal */
export const IBCChannelIDShape = M.string();

/** @internal */
export const IBCChannelInfoShape = M.splitRecord({
portId: M.string(),
Expand All @@ -63,8 +62,10 @@ export const IBCChannelInfoShape = M.splitRecord({
state: M.scalar(), // XXX
version: M.string(),
});

/** @internal */
export const IBCConnectionIDShape = M.string();

/** @internal */
export const IBCConnectionInfoShape = M.splitRecord({
id: IBCConnectionIDShape,
Expand Down Expand Up @@ -115,15 +116,18 @@ export const DenomInfoShape = {
brand: M.or(M.remotable('Brand'), M.undefined()),
baseDenom: M.string(),
};
harden(DenomInfoShape);

/** @type {TypedPattern<DenomAmount>} */
export const DenomAmountShape = { denom: DenomShape, value: M.nat() };
harden(DenomAmountShape);

/** @type {TypedPattern<Amount<'nat'>>} */
export const AnyNatAmountShape = harden({
export const AnyNatAmountShape = {
brand: M.remotable('Brand'),
value: M.nat(),
});
};
harden(AnyNatAmountShape);

/** @type {TypedPattern<AmountArg>} */
export const AmountArgShape = M.or(AnyNatAmountShape, DenomAmountShape);
Expand Down Expand Up @@ -152,10 +156,11 @@ export const ICQMsgShape = M.splitRecord(
export const TypedJsonShape = M.splitRecord({ '@type': M.string() });

/** @see {Chain} */
export const chainFacadeMethods = harden({
export const chainFacadeMethods = {
getChainInfo: M.call().returns(VowShape),
makeAccount: M.call().returns(VowShape),
});
};
harden(chainFacadeMethods);

/**
* for google/protobuf/timestamp.proto, not to be confused with TimestampShape
Expand All @@ -165,6 +170,7 @@ export const chainFacadeMethods = harden({
* string
*/
export const TimestampProtoShape = { seconds: M.string(), nanos: M.number() };
harden(TimestampProtoShape);

/**
* see {@link TxBody} for more details
Expand All @@ -187,6 +193,5 @@ export const TxBodyOptsShape = M.splitRecord(
*/
export const AnyNatAmountsRecord = M.and(
M.recordOf(M.string(), AnyNatAmountShape),
M.not(harden({})),
M.not({}),
);
harden(AnyNatAmountsRecord);
Loading

0 comments on commit 47bebb8

Please sign in to comment.