fix(compilers/openapi): keep format: password at every position

compilers/openapi/conformance_test.go

@@ -176,6 +176,7 @@ func conformanceCases() []conformanceCase {
 		{"enum-numeric", assertEnumNumeric, []string{"enums-numeric"}},
 		{"empty-enum", assertEmptyEnum, nil},
 		{"scalar-format", assertScalarFormat, []string{"custom-scalars"}},
+		{"secret-format", assertSecretFormat, nil},
 		{"encoding-byte", assertEncodingByte, []string{"encoding-hints"}},
 		{"content-vocabulary", assertContentVocabulary, []string{"encoding-hints"}},
 		{"xml-hints", assertXMLHints, nil},
@@ -1126,11 +1127,141 @@ func assertScalarFormat(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	require.NotNil(t, sc.Constraints.MinLength)
 	assert.Equal(t, int64(4), *sc.Constraints.MinLength)
 
-	// format: password is a redaction request about a use of the value, so it
-	// lands on the property rather than on the shared encoding node.
+	// format: password is a redaction request about how the value is handled, so
+	// it hoists a Scalar of its own carrying Sensitive and the verbatim spelling,
+	// and the property that writes it keeps Property.Secret beside that node.
 	token, ok := propByWire(h, "token")
 	require.True(t, ok)
-	assert.True(t, token.Secret)
+	assert.True(t, token.Secret, "the property that writes it is secret at the use")
+	secret, ok := doc.Types[token.Type.Target].(*ir.Scalar)
+	require.True(t, ok, "password no longer stays on the shared primitive")
+	assert.True(t, secret.Sensitive, "whole-type redaction")
+	require.NotNil(t, secret.Encoding)
+	assert.Equal(t, "password", secret.Encoding.Name, "the source token verbatim")
+	require.NotNil(t, secret.Base)
+	assert.Equal(t, ir.TypeID("t/prim/string"), secret.Base.Target, "over the bare type's primitive")
+}
+
+// assertSecretFormat pins where a `format: password` position's fact lands: a
+// Scalar of its own carrying Sensitive, the verbatim spelling and the bounds
+// written beside it at every scalar position, and Property.Secret wherever a
+// property or a header is the carrier, through any wrapper. It reads the nodes
+// rather than the golden's bytes, so a node that moved to another position
+// while keeping the same fields still fails (GitHub #579).
+func assertSecretFormat(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
+	// The named component every reference resolves to, carrying its own bound.
+	requireSecretScalar(t, doc, namedID("Pw"), "component schema")
+	requireLengthBounds(t, doc, namedID("Pw"), new(int64(8)), nil, "component schema")
+
+	login, ok := doc.Types[namedID("Login")].(*ir.Model)
+	require.True(t, ok, "Login is a model")
+
+	// Inline property: Property.Secret beside the hoisted node.
+	inline, ok := propByWire(login, "inline")
+	require.True(t, ok)
+	assert.True(t, inline.Secret, "the inline property is secret")
+	requireSecretScalar(t, doc, inline.Type.Target, "inline property schema")
+
+	// Property via $ref: Secret from the referent, which carries the node.
+	viaRef, ok := propByWire(login, "viaRef")
+	require.True(t, ok)
+	assert.True(t, viaRef.Secret, "a $ref to a redaction schema is secret at the use")
+	assert.Equal(t, namedID("Pw"), viaRef.Type.Target, "it resolves to the component")
+
+	// The wrappers: each reaches the Sensitive node through a nullable ref or a
+	// Base chain, and each use is secret all the same.
+	for _, wire := range []string{"nullable", "wrapped", "viaAlias"} {
+		p, ok := propByWire(login, wire)
+		require.True(t, ok, wire)
+		assert.True(t, p.Secret, "%s: a use reaching the node through a wrapper is secret", wire)
+	}
+
+	// Array items.
+	arr, ok := propByWire(login, "arr")
+	require.True(t, ok)
+	list, ok := doc.Types[arr.Type.Target].(*ir.List)
+	require.True(t, ok, "arr hoists a list")
+	requireSecretScalar(t, doc, list.Elem.Target, "array items")
+	requireLengthBounds(t, doc, list.Elem.Target, nil, new(int64(64)), "array items")
+
+	// additionalProperties and patternProperties.
+	bag, ok := doc.Types[namedID("Bag")].(*ir.Model)
+	require.True(t, ok, "Bag is a model")
+	require.NotNil(t, bag.AdditionalProps)
+	requireSecretScalar(t, doc, bag.AdditionalProps.Value.Target, "additionalProperties")
+	require.Len(t, bag.AdditionalProps.Patterns, 1)
+	requireSecretScalar(t, doc, bag.AdditionalProps.Patterns[0].Value.Target, "patternProperties")
+
+	// prefixItems.
+	pair, ok := doc.Types[namedID("Pair")].(*ir.Tuple)
+	require.True(t, ok, "prefixItems hoists a tuple")
+	require.Len(t, pair.Elems, 1)
+	requireSecretScalar(t, doc, pair.Elems[0].Target, "prefixItems")
+
+	op, ok := opByName(doc, "login")
+	require.True(t, ok)
+
+	// A header parameter's schema, inline, and a query parameter's, via $ref.
+	token, ok := paramByName(op, "X-Token")
+	require.True(t, ok)
+	requireSecretScalar(t, doc, token.Type.Target, "header parameter schema")
+	requireLengthBounds(t, doc, token.Type.Target, new(int64(12)), nil, "header parameter schema")
+	key, ok := paramByName(op, "api_key")
+	require.True(t, ok)
+	assert.Equal(t, namedID("Pw"), key.Type.Target, "the query parameter resolves to the component")
+
+	// Request and response content schemas.
+	requireSecretScalar(t, doc, openapitest.BodyTarget(t, op.Request), "request content schema")
+	require.Len(t, op.Responses, 1)
+	requireSecretScalar(t, doc, openapitest.BodyTarget(t, op.Responses[0].Payload), "response content schema")
+
+	// A response header: Property.Secret beside the node, and through a
+	// nullable reference to the component.
+	require.Len(t, op.Responses[0].Headers, 2)
+	header := op.Responses[0].Headers[0]
+	assert.True(t, header.Secret, "the response header is secret")
+	requireSecretScalar(t, doc, header.Type.Target, "response header schema")
+	maybe := op.Responses[0].Headers[1]
+	assert.True(t, maybe.Secret, "a nullable reference to a redaction schema is secret")
+	assert.Equal(t, ir.TypeRef{Target: namedID("Pw"), Nullable: true}, maybe.Type)
+
+	// The guard: a bare string still targets the shared primitive, which stays
+	// not sensitive — the redaction must never leak onto the node every
+	// declaration of the type shares (invariant 3).
+	plain, ok := propByWire(login, "plain")
+	require.True(t, ok)
+	assert.Equal(t, ir.TypeID("t/prim/string"), plain.Type.Target,
+		"a plain string keeps the shared primitive")
+	assert.False(t, plain.Secret, "and asks for no redaction")
+	prim, ok := doc.Types[ir.TypeID("t/prim/string")]
+	require.True(t, ok, "the shared string primitive is registered")
+	assert.False(t, prim.Common().Sensitive, "the shared primitive is never sensitive")
+}
+
+// requireSecretScalar requires the node at id to be the Scalar a
+// `format: password` position hoists, over the bare type's own primitive.
+func requireSecretScalar(t *testing.T, doc *ir.Document, id ir.TypeID, position string) {
+	t.Helper()
+	node, ok := doc.Types[id]
+	require.True(t, ok, "%s: nothing is interned at %s", position, id)
+	sc, ok := node.(*ir.Scalar)
+	require.True(t, ok, "%s: the position hoists a Scalar of its own", position)
+	assert.True(t, sc.Sensitive, "%s: whole-type redaction", position)
+	require.NotNil(t, sc.Encoding, "%s: the format reaches Encoding", position)
+	assert.Equal(t, "password", sc.Encoding.Name, "%s: the source token verbatim", position)
+	require.NotNil(t, sc.Base, "%s: a base to ride", position)
+	assert.Equal(t, ir.TypeID("t/prim/string"), sc.Base.Target, "%s: over the bare type's primitive", position)
+}
+
+// requireLengthBounds requires the Scalar at id to carry the length bounds the
+// position wrote beside its format: a hoisting format must not drop them.
+func requireLengthBounds(t *testing.T, doc *ir.Document, id ir.TypeID, minLen, maxLen *int64, position string) {
+	t.Helper()
+	sc, ok := doc.Types[id].(*ir.Scalar)
+	require.True(t, ok, "%s: a Scalar at %s", position, id)
+	require.NotNil(t, sc.Constraints, "%s: the bounds written beside the format", position)
+	assert.Equal(t, minLen, sc.Constraints.MinLength, "%s: minLength", position)
+	assert.Equal(t, maxLen, sc.Constraints.MaxLength, "%s: maxLength", position)
 }
 
 // assertXMLHints covers the XML wire shape, whose hints attach at two carriers

compilers/openapi/internal/merge/merge.go

@@ -300,6 +300,10 @@ const maxTypeResolveDepth = 64
 func (g *Merger) recordRedeclarationConflict(dst, src *ir.Property) (dropped, lost bool) {
 	pointer := src.Provenance.Pointer
 	if dst.Type.Target != src.Type.Target {
+		// A format hoist alone differs here: `{type: string, format: password}`
+		// owns a Scalar while a bare string stays on the shared primitive, so the
+		// later declaration is dropped whole, its shape details with it, and which
+		// one that is depends on branch order. Nothing reports it (GitHub #782).
 		dropped = true
 	} else {
 		// Same referent, and the only thing left for the two to disagree about

compilers/openapi/internal/operation/params.go

@@ -125,6 +125,12 @@ func reservedHeaderParamDiag(c lowering.Ctx, name string, in soa.ParameterIn, pp
 // parameter, whose media type goes on the binding. Constraints come from that
 // same schema position; the default comes from it too, falling back to its $ref
 // target (§14).
+//
+// `format: password` on the parameter's schema hoists a Scalar carrying
+// Sensitive. ir.Parameter has no redaction field (GitHub #578 owns one), so the
+// type is the only carrier, and through a wrapper — an alias component, an
+// allOf over one $ref — the parameter's own node is not Sensitive: only the node
+// its Base chain reaches is.
 func fillParamType(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, param *ir.Parameter, binding *ir.HTTPParamBinding, p *soa.Parameter, pptr jsontext.Pointer, name string) []ir.Diagnostic {
 	elected, diags := electTypeSpelling(c, p.GetSchema(), p.GetContent(), p.GetRootNode(), pptr)
 	paramType, typeDiags := schema.CarriedRef(c, ts, anchors, schema.TopLevelDepth, elected.js, elected.pointer, name)

compilers/openapi/internal/schema/compose_test.go

@@ -90,7 +90,11 @@ func TestAllOf_ReconcileAccumulatesRicherDetailWhateverTheOrder(t *testing.T) {
 	t.Parallel()
 	// The bare declaration comes first and the richer one second: reconciliation
 	// must still surface every optional detail, so branch order never loses
-	// information (the reverse of the forkee documented-first shape).
+	// information (the reverse of the forkee documented-first shape). The richer
+	// branch writes no format because a format that hoists a node of its own
+	// changes the branch's type target, and the fold then drops the declaration
+	// rather than folding its details (GitHub #782). The secrecy half of that is
+	// TestAllOf_RedeclarationOrsSecret.
 	spec := openapitest.ComponentSpec(`    Tokenish:
       allOf:
         - type: object
@@ -101,7 +105,6 @@ func TestAllOf_ReconcileAccumulatesRicherDetailWhateverTheOrder(t *testing.T) {
           properties:
             token:
               type: string
-              format: password
               description: the access token
               default: none
               minLength: 1
@@ -117,7 +120,6 @@ func TestAllOf_ReconcileAccumulatesRicherDetailWhateverTheOrder(t *testing.T) {
 
 	tok := m.Properties[0]
 	assert.True(t, tok.Required, "required in the richer branch => required on the merged property")
-	assert.True(t, tok.Secret, "secrecy (format:password) adopted from the richer branch")
 	assert.Equal(t, "the access token", tok.Docs.Description, "description adopted whichever branch carries it")
 	require.NotNil(t, tok.Default, "default adopted from the richer branch")
 	require.NotNil(t, tok.Constraints, "constraints adopted from the richer branch")
@@ -127,6 +129,46 @@ func TestAllOf_ReconcileAccumulatesRicherDetailWhateverTheOrder(t *testing.T) {
 	require.Len(t, tok.Examples, 1, "examples adopted from the richer branch")
 }
 
+// TestAllOf_RedeclarationOrsSecret pins the one property field a redeclaration
+// OR-s rather than folds: a redaction requested by either branch survives the
+// merge whichever declaration the type fold keeps (reconcileProperty). The two
+// branches no longer share a target once one of them writes `format: password`,
+// which hoists a Scalar of its own, so the shape fold drops the losing
+// declaration and keeps it verbatim rather than folding its details, silently
+// and in an order-dependent way (GitHub #782). Once that is fixed, this case
+// should assert the folded fields as well.
+func TestAllOf_RedeclarationOrsSecret(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct{ name, branches string }{
+		{"password first", `        - type: object
+          properties:
+            token: {type: string, format: password}
+        - type: object
+          properties:
+            token: {type: string}
+`},
+		{"password second", `        - type: object
+          properties:
+            token: {type: string}
+        - type: object
+          properties:
+            token: {type: string, format: password}
+`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			spec := openapitest.ComponentSpec("    Tokenish:\n      allOf:\n" + tc.branches)
+			doc, diags := lowerSpec(t, spec)
+			openapitest.RequireNoErrorDiags(t, diags)
+			m, ok := doc.Types[componentID("Tokenish")].(*ir.Model)
+			require.True(t, ok, "Tokenish should be a model")
+			require.Len(t, m.Properties, 1, "token reconciles to a single property")
+			assert.True(t, m.Properties[0].Secret,
+				"either branch's format:password makes the merged field secret, %s", tc.name)
+		})
+	}
+}
+
 func TestAllOf_ConflictingRedeclaredDescriptionDiagnosed(t *testing.T) {
 	t.Parallel()
 	// Two branches describe the same field differently. The first declaration in

compilers/openapi/internal/schema/hoist_internal_test.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -99,6 +100,29 @@ func TestRegisteredNode_ReportsAMissRatherThanDroppingIt(t *testing.T) {
 	assert.Equal(t, id, td.Common().ID)
 }
 
+// TestFormatTable_EveryRowDistinguishesTheBareType pins the invariant the
+// `string/password` row broke (GitHub #579): a (type, format) key may only map
+// to a primitive kind the pairing itself distinguishes, because a row mapping to
+// the kind the bare type selects makes the pairing unreadable — the position
+// resolves to the shared primitive and nothing anywhere records the format. A
+// format whose kind does not differ must be absent from the table and hoist a
+// node instead, as byte, password and an unknown format do.
+func TestFormatTable_EveryRowDistinguishesTheBareType(t *testing.T) {
+	t.Parallel()
+	for key, prim := range formatTable {
+		typ, _, paired := strings.Cut(key, "/")
+		if !paired {
+			continue // the bare type's own row, which no format selects
+		}
+		bare, ok := formatTable[typ]
+		require.True(t, ok, "row %q names a type with no row of its own", key)
+		assert.NotEqual(t, bare, prim,
+			"row %q maps to the same kind (%s) as the bare type %q, so the pairing "+
+				"reaches no field once the position resolves to the shared primitive; "+
+				"a format whose kind does not differ must hoist a node instead", key, prim, typ)
+	}
+}
+
 // TestRegisteredNode_LeavesANodeStillBeingBuiltToItsBuilder pins the one miss
 // that is no bug: a revisit while the node's build is still running further up
 // the walk. That frame finishes the node once its build returns, so the