Skip to content

feat(contract-toolkit): let a contract emit additional and legacy-shaped credential configs [FND-4755] - #4794

Merged
cmgrote merged 4 commits into
mainfrom
christophergrote/fnd-4755
Nov 6, 2026
Merged

cmgrote merged 4 commits into
mainfrom
christophergrote/fnd-4755

Conversation

@cmgrote

@cmgrote cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

An App.pkl contract could emit only one credential configmap, <connectorConfigName>.json, and it was always wrapped. That left two shapes in the field inexpressible:

  • a second credential type selected by the same entrypoint's form;
  • a legacy credential type whose form lacks the envelope keys or the hidden identity properties. Two examples: the @atlan/connectors csa-connectors-objectstore form (bare {"config": ...}) and generic-miner's Credential.pkl object-store form (envelope, no identity properties).

This PR adds:

  • additionalCredentialConfigs: Listing<CredentialConfig>. Each entry is rendered by the same renderer as the primary config. That renderer is refactored to take a spec, and the primary is now built from the top-level credential* properties. The primary's output is byte-identical; a differential eval old vs new checked this, including the JDBC credentialUrlGroup path and every credential* knob.
  • CredentialConfig.wrapper / .identityProperties (both default true). They independently drop the envelope keys (icon, helpdeskLink, logo, connector, defaultConnectorType) and the hidden name / connector / connectorType properties. Neither is neutral for an existing credential type:
    • the SDK configmap route returns defaultConnectorType to the frontend;
    • the frontend submits the hidden properties in every credential body.
  • AuthOption.nestedHelp / nestedWidth. Both are optional and omitted when null.
  • Bundles hoist and dedupe additional configs like the primary.
  • Refused at generation:
    • declared on a bundle root (it would be silently dropped);
    • no uiConfig;
    • a name that is not atlan-connectors- / csa-connectors- plus kebab-case. These are the SDK's credential-template stems (is_form_configmap), so any other stem can be served as the setup form, and / or .. would write outside app/generated/;
    • duplicate or colliding names;
    • a wrapped entry with no connector;
    • a field that the entry's flags make unused.

New example examples/credential-configs/ and tests/additional_credential_configs_test.pkl (19 tests). README, docs/reference.md and CLAUDE.md are updated.

Fixes FND-4755. This unblocks FND-4752 (csa-uber onto App.pkl). In that repo, the orphaned ...-asset-export-advanced-delivery.json should be deleted: nothing has referenced it since csa-uber dfc7065. The App.pkl half of generic-miner's deferred credential redesign (FND-4741) is also covered.

Security-relevant

  • Configmap names are now restricted to the SDK's credential-template prefixes plus kebab-case. This prevents path traversal out of app/generated/ and stops a credential template from being served as a setup form.

Test plan

  • ./scripts/regenerate-all.sh: no drift in existing examples
  • ./scripts/check-invariants.sh
  • pkl test tests/*.pkl: 634 passed
  • test-sdk-import.py: 63 files import
  • check-ruff-stable.py: 0 unstable
  • The bare object-store config built with the new API matches csa-uber's committed csa-connectors-objectstore.json, apart from two no-op keys (required: false on panes, help: "" on the radio)
  • Two independent reviews (correctness, API design). Their findings are fixed in the second commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AbKqGZBmKKFUgJnacX4QB1

…ntial configs [FND-4755]

App.pkl emitted exactly one credential configmap per contract,
<connectorConfigName>.json, always wrapped in the Atlan credential envelope.
Two shapes in the field could not be expressed: a second credential type
selected by the same entrypoint's form, and a legacy wrapper-less config
such as the @atlan/connectors csa-connectors-objectstore form.

- New CredentialConfig class and additionalCredentialConfigs listing. Each
  entry renders through the same renderer as the primary config, which is
  refactored to take a spec; the primary is built from the top-level
  credential* properties and its output is byte-identical.
- CredentialConfig.bare emits only {"config": {properties, anyOf}}: no
  wrapper keys and no hidden name/connector/connectorType properties. The
  wrapper is not neutral for a legacy type: the SDK configmap route returns
  defaultConnectorType, and the hidden properties land in every credential
  body the form submits.
- AuthOption gains optional nestedHelp / nestedGrid (omitted when null).
- Bundles hoist and dedupe additional configs like the primary.
- Generation is refused for a missing uiConfig, a name without the
  atlan-connectors-/csa-connectors- prefix (the SDK's credential-template
  rule; any other stem can be served as the setup form), duplicate or
  colliding names, and a wrapped entry with no connector.

New example examples/credential-configs and tests. A bare object-store
config built with it matches the committed csa-connectors-objectstore.json
up to two no-op keys (required: false on panes, help: "" on the radio).

Claude-Session: https://claude.ai/code/session_01AbKqGZBmKKFUgJnacX4QB1
… close review gaps [FND-4755]

Design review: `bare` coupled two independent choices. generic-miner's
Credential.pkl object-store config keeps the envelope (incl.
defaultConnectorType, which the SDK route returns) but has no hidden
identity properties, a combination `bare` could not express.

- Replace `bare` with `wrapper` (envelope keys) and `identityProperties`
  (hidden name/connector/connectorType), both default true.
- Refuse fields a flag makes unused instead of ignoring them.
- Rename AuthOption.nestedGrid -> nestedWidth, matching authWidth / width.

Correctness review:
- A bundle root's additionalCredentialConfigs were silently dropped (only
  single-entrypoint mode emits them). Now refused with a pointer to the
  entrypoint contracts.
- Names were checked for prefix only, so `csa-connectors-x/../../evil`
  wrote outside app/generated/. Names must now be the prefix plus
  lowercase kebab-case.

Docs point Credential.pkl + additionalOutputFiles users at the new API.

Claude-Session: https://claude.ai/code/session_01AbKqGZBmKKFUgJnacX4QB1
@linear

linear Bot commented Nov 6, 2026

Copy link
Copy Markdown

FND-4755

@cmgrote

cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

/lens

@atlan-app-fleet

atlan-app-fleet Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

lens summary · updated after round 2 (re-review · only commits since b84a4e8)

✅ Ready to merge — every finding is resolved

Open findings: 🔴 0 critical · 🟠 0 high · 🟡 0 medium · ⚪ 0 low (nit)

lens reads this PR as — A single App.pkl entrypoint could emit only its primary wrapped credential configmap, so it could not express another credential type selected by the same form or reproduce legacy credential templates with missing envelope or identity fields. How: The PR adds CredentialConfig entries for additional configmaps, renders them through the same credential renderer as the primary with independent wrapper and identity-property switches, and hoists/deduplicates them in bundles with generation-time placement and naming checks.

Approach check — ✅ every concern has been addressed.

  • ✔️ The unused-field refusal guarantee is incomplete (A1) — addressed in round 2
✔️ Resolved (3)
id severity where finding found resolved how
F-1f422c 🟡 medium contract-toolkit/docs/reference.md:1992 Disabled connectorType and namePlaceholder overrides are not rejected round 1 round 2 its code was removed or rewritten
F-86c1ee 🟡 medium contract-toolkit/src/App.pkl:5633 Add a CLI invariant case for the new eval-time guard round 1 round 2 verified fixed
F-ac04d8 🟡 medium contract-toolkit/src/App.pkl:5586 Reject fields made unused by both flags round 1 round 2 its code was removed or rewritten
🕘 Round history (2)
round run commits new resolved blocking open cost log
1 first review 468e2b1..b84a4e8 3 0 0 $0.052 run
2 re-review · only commits since b84a4e8 b84a4e8..c239a8a 0 3 0 $0.024 run

round 2 · $0.076 of $1.00 · 51 model calls · 0 failed requests · 83% prompt cache hits · gpt-6-luna

…r flag is off [FND-4755]

lens F-ac04d8 / F-1f422c: with wrapper = false a custom connectorType, and
with identityProperties = false a custom namePlaceholder, were dropped
silently, contradicting the documented refusal of flag-disabled fields.
Both are now nullable (renderer defaults "rest" / "Host Name"), so an
explicit setting is detectable exactly and refused.

lens F-86c1ee: check-invariants.sh gains a section for the
additionalCredentialConfigs guard (path-traversal name, both
flag-disabled fields, plus a generating control). Verified red against
the previous guard.

Claude-Session: https://claude.ai/code/session_01AbKqGZBmKKFUgJnacX4QB1
@cmgrote

cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

/lens

@atlan-app-fleet

Copy link
Copy Markdown
Contributor

✅ lens · round 2 · re-review · only commits since b84a4e8 — ready to merge — every finding resolved · $0.08

Open findings: 🔴 0 critical · 🟠 0 high · 🟡 0 medium · ⚪ 0 low (nit)
Resolved this round: 3

Approach check — ✅ concerns addressed

  • Problem: A single App.pkl entrypoint could emit only its primary wrapped credential configmap, so it could not express another credential type selected by the same form or reproduce legacy credential templates with missing envelope or identity fields.
  • How the PR solves it: The PR adds CredentialConfig entries for additional configmaps, renders them through the same credential renderer as the primary with independent wrapper and identity-property switches, and hoists/deduplicates them in bundles with generation-time placement and naming checks.
  • ✔️ The unused-field refusal guarantee is incomplete (A1) — addressed in round 2

$0.076 of $1.00 · 51 model calls · 0 failed requests

Full summary · Run log

@atlan-ci atlan-ci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lens: ready to merge — every finding at every level is resolved (round 2).

@cmgrote
cmgrote added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit ceff3a7 Nov 6, 2026
34 checks passed
@cmgrote
cmgrote deleted the christophergrote/fnd-4755 branch November 6, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants