Skip to content

πŸŽ™οΈ task - fix(iam): single-element condition array permadrifts β€” never converges to KEEPΒ #89

Description

@ehm-a-seaturtle

πŸ¦«πŸŽ™οΈ dispatch to foreman

πŸ’§ task enqueued
   β”œβ”€ priority = ?
   β”œβ”€ yieldage = ?
   └─ leverage = ?

title
fix(iam): single-element condition array permadrifts β€” never converges to KEEP
description

.what

a single-element IAM condition value array can never converge. a wish that declares

condition: { StringEquals: { "token.actions.githubusercontent.com:actor_id": ["10381896"] } }

plans UPDATE on the role forever β€” no apply can settle it.

.why

AWS collapses a one-member condition array into a bare scalar when it stores the policy, and returns the scalar on every read. so:

side shape
desired (the wish) ["10381896"]
remote (GetRole -> castIntoDeclaredAwsIamPolicyStatement) "10381896"

declastructs computeChange decides equality by serialize(omitReadonly(remote)) === serialize(omitReadonly(desired)). serialize sorts keys but does not treat ["x"] and "x" as equal, so the compare is false on every plan.

to AWS these are the SAME grant (a one-member OR set). the compare must agree, or the resource is permanently dirty.

castIntoDeclaredAwsIamPolicyStatement.js passes the condition through raw:

...(stmt.Condition !== undefined && { condition: stmt.Condition }),

.the seam that matters

the fix must sit where BOTH sides pass, not in the cast alone. the cast only touches the remote; the desired is hand-authored in the wish and reaches computeChange untouched by this package.

that seam is the DeclaredAwsIamPolicyStatement constructor β€” a wish calls new DeclaredAwsIamPolicyStatement({...}), and the cast calls .as({...}), which is .build -> new this(props). one override covers both.

.the repair, demonstrated

verified against declastruct-aws@1.10.4 in ahbode/infrastructure, patched in node_modules, red/green proven.

new src/domain.objects/asCanonicalIamPolicyCondition.ts

/**
 * .what = canonicalizes an iam policy condition map into the ONE form aws stores
 * .why = aws collapses a single-element condition value array into a bare scalar
 *   when it stores the policy, and returns the scalar on every read. `["x"]` and
 *   `"x"` are the SAME grant to aws (a one-member OR set), so the compare must
 *   treat them as the same too. we canonicalize toward the scalar because that is
 *   aws own fixed point: unwrap once and every later read agrees.
 * .note = only a SINGLE-element array is unwrapped. a multi-element array is a real
 *   OR set and stays an array; an empty array is left untouched.
 */
export const asCanonicalIamPolicyCondition = (condition: any): any => {
  if (!condition || typeof condition !== "object" || Array.isArray(condition))
    return condition;
  return Object.entries(condition).reduce((acc: any, [operator, claims]: any) => {
    if (!claims || typeof claims !== "object" || Array.isArray(claims)) {
      acc[operator] = claims;
      return acc;
    }
    acc[operator] = Object.entries(claims).reduce((claimAcc: any, [claim, value]: any) => {
      claimAcc[claim] = Array.isArray(value) && value.length === 1 ? value[0] : value;
      return claimAcc;
    }, {});
    return acc;
  }, {});
};

edit src/domain.objects/DeclaredAwsIamPolicyStatement.ts

export class DeclaredAwsIamPolicyStatement extends DomainLiteral<DeclaredAwsIamPolicyStatement> {
  /**
   * .what = canonicalizes `condition` before the props reach the base constructor
   * .why = the plan compares a hand-authored DESIRED statement against a REMOTE one
   *   cast from the aws api, and aws collapses a single-element condition value
   *   array into a scalar on store. `serialize` sees `["x"] !== "x"`, so the role
   *   reads UPDATE on every plan and can never converge.
   *   the fix belongs HERE, at construction, because it is the one seam BOTH sides
   *   pass through. to canonicalize only in the cast would fix the remote side
   *   alone and leave the drift standing.
   */
  constructor(props: any, options?: any) {
    super(
      props?.condition !== undefined
        ? { ...props, condition: asCanonicalIamPolicyCondition(props.condition) }
        : props,
      options,
    );
  }
  public static nested = { /* unchanged */ };
}

.the clamp (proven to bite)

given("[case1] the actor_id allowlist, as one id", () => {
  const desired = new DeclaredAwsIamPolicyStatement({
    effect: "Allow",
    action: "sts:AssumeRoleWithWebIdentity",
    condition: { StringEquals: { "token.actions.githubusercontent.com:actor_id": ["10381896"] } },
  });
  const remote = new DeclaredAwsIamPolicyStatement({
    effect: "Allow",
    action: "sts:AssumeRoleWithWebIdentity",
    condition: { StringEquals: { "token.actions.githubusercontent.com:actor_id": "10381896" } },
  });
  when("[t0] the plan compares them by serialize", () => {
    then("they are equal, so the change reads KEEP not UPDATE", () => {
      expect(serialize(desired)).toEqual(serialize(remote));
    });
  });
});

plus [case2] a two-id array stays an array (a real OR set is never collapsed), and [case3] an already-scalar value passes through untouched.

teeth verified: with the constructor reverted to a bare super(props, options), [case1] goes RED while [case2]/[case3] stay green. restored -> all 3 green.

.companion defect (different repo)

the same plan prints phantom key-order diff lines on nested condition maps. that half lives in declastruct core (getDisplayableDiff.compareKeys returns Infinity - Infinity = NaN for nested keys), not here. dispatched separately to ehmpathy/declastruct.

.why it matters

this is not cosmetic. a resource that can never read KEEP:

  • makes every plan noisy, so a REAL change hides in the churn
  • burns an apply on every run, which for an IAM trust policy is a live-credentials write
  • trains the operator to skim the plan β€” the exact habit a declarative tool exists to prevent

.repro

any wish with a one-element condition array. ours: provision/aws.auth/account=prod/resources.oidc.ts, the production-on-else-apply statement (the actor_id human allowlist, written as an array on purpose so a second id is a one-token edit).

the wish must NOT be bent to the wires shape to dodge this β€” the library should converge on either equivalent form.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions