@W-23895509: Block secret values in Site preferences

.claude/skills/generate-site-preferences-impex/SKILL.md

@@ -19,7 +19,7 @@ Use proactively whenever:
 - Creating a new commerce app (always needs preferences)
 - User mentions "configuration", "settings", or "preferences"
 - App needs merchant-configurable options
-- Adding API credentials, feature toggles, or environment selection
+- Adding feature toggles, environment selection, or other non-secret merchant settings
 - Any scenario where merchants need to configure app behavior
 
 ## Step 1: Collect preference information
@@ -39,14 +39,14 @@ Use proactively whenever:
 
 Read `references/attribute-types.md` for complete type reference with examples:
 - **boolean** - Enable/disable flags
-- **string** - API keys, URLs, short text
+- **string** - URLs, public identifiers, short text
 - **text** - Long descriptions, JSON configs
 - **integer** - Counts, limits
 - **decimal** - Percentages, ratings
 - **enum-of-string** - Dropdown selections
 - **set-of-string** - Multiple selections
 - **email** - Email addresses
-- **password** - Encrypted secrets
+- **password** - Legacy masked values only; do not use Site preferences for secret values
 
 ## Step 3: Use app-specific patterns
 
@@ -80,24 +80,6 @@ Create the attribute definitions file:
                 <default-value>false</default-value>
             </attribute-definition>
 
-            <!-- API Credentials -->
-            <attribute-definition attribute-id="{appName}ApiKey">
-                <display-name xml:lang="x-default">{displayName} API Key</display-name>
-                <type>string</type>
-                <mandatory-flag>false</mandatory-flag>
-                <externally-managed-flag>false</externally-managed-flag>
-                <min-length>0</min-length>
-                <default-value></default-value>
-            </attribute-definition>
-
-            <attribute-definition attribute-id="{appName}ApiSecret">
-                <display-name xml:lang="x-default">{displayName} API Secret</display-name>
-                <type>password</type>
-                <mandatory-flag>false</mandatory-flag>
-                <externally-managed-flag>false</externally-managed-flag>
-                <min-length>0</min-length>
-            </attribute-definition>
-
             <!-- Environment Selection -->
             <attribute-definition attribute-id="{appName}Environment">
                 <display-name xml:lang="x-default">{displayName} Environment</display-name>
@@ -133,8 +115,6 @@ Create the attribute definitions file:
             <attribute-group group-id="{appName}">
                 <display-name xml:lang="x-default">{displayName}</display-name>
                 <attribute attribute-id="{appName}Enabled"/>
-                <attribute attribute-id="{appName}ApiKey"/>
-                <attribute attribute-id="{appName}ApiSecret"/>
                 <attribute attribute-id="{appName}Environment"/>
                 <attribute attribute-id="{appName}DebugMode"/>
             </attribute-group>
@@ -157,7 +137,6 @@ Create default preference values:
     <preference preference-id="{appName}Environment">sandbox</preference>
     <preference preference-id="{appName}DebugMode">false</preference>
 
-    <!-- Leave sensitive values (API keys, secrets) empty -->
 </preferences>
 ```
 
@@ -177,10 +156,11 @@ For international merchants, add translations. See `references/attribute-types.m
 ### Defaults
 - Safe defaults: Start with features disabled
 - Sensible values: Choose defaults that work for most cases
-- Empty credentials: Don't include placeholder API keys
+- Secret values: Do not configure them as Site preferences
 
 ### Security
-- Use `password` type for API secrets
+- Never generate Site preferences for keys, tokens, passwords, credentials, or other secret values
+- Use ecom service credentials and `LocalServiceRegistry` for authentication secrets
 - Don't expose secrets in XML
 - Document sensitive fields clearly
 
@@ -197,7 +177,7 @@ For international merchants, add translations. See `references/attribute-types.m
 - [ ] Descriptions provide helpful guidance
 - [ ] Appropriate data types
 - [ ] Safe default values
-- [ ] Password type for secrets
+- [ ] No secret values stored in Site preferences
 - [ ] All attributes in group definition
 - [ ] XML well-formed
 - [ ] SITEID placeholder used
@@ -225,7 +205,7 @@ xmllint --noout impex/install/sites/SITEID/preferences.xml
 | No default values | Set sensible defaults |
 | Everything mandatory | Only require essentials |
 | Generic descriptions | Provide clear guidance |
-| Hardcoded credentials | Leave empty for merchants |
+| Secret values in Site preferences | Remove them; use ecom service credentials for authentication secrets |
 | Not in group | Add all attributes to group |
 
 ## Quick reference

.claude/skills/generate-site-preferences-impex/evals/evals.json

@@ -1,9 +1,9 @@
 {
   "evals": [
     {
-      "prompt": "Merchants need to configure API keys and select sandbox vs production. Can you create the site preferences?",
-      "context": "User needs configurable merchant settings",
-      "expected_behavior": "Should trigger generate-site-preferences-impex skill, create attribute definitions with API key (string), secret (password), environment (enum)"
+      "prompt": "Merchants need to select sandbox vs production and enable the integration. Can you create the site preferences?",
+      "context": "User needs non-secret configurable merchant settings",
+      "expected_behavior": "Should trigger generate-site-preferences-impex skill and create environment (enum) and enabled (boolean) attributes; no secret values are stored in Site preferences, and authentication secrets remain in ecom service credentials"
     },
     {
       "prompt": "I need to add some settings that merchants can configure in Business Manager for my tax app",

.claude/skills/generate-site-preferences-impex/references/app-patterns.md

@@ -45,6 +45,11 @@ This reference contains pre-built site preference patterns for common commerce a
 
 ## Payment App Preferences
 
+Secret values are intentionally absent from this pattern. Do not store keys, tokens,
+passwords, or credentials in Site preferences. Configure authentication secrets as ecom service
+credentials and read them through `LocalServiceRegistry`. Public identifiers such as a
+publishable key may remain Site preferences.
+
 ```xml
 <custom-attribute-definitions>
     <attribute-definition attribute-id="paymentEnabled">
@@ -65,12 +70,6 @@ This reference contains pre-built site preference patterns for common commerce a
         <min-length>0</min-length>
     </attribute-definition>
 
-    <attribute-definition attribute-id="paymentSecretKey">
-        <display-name xml:lang="x-default">Secret API Key</display-name>
-        <type>password</type>
-        <min-length>0</min-length>
-    </attribute-definition>
-
     <attribute-definition attribute-id="paymentTestMode">
         <display-name xml:lang="x-default">Test Mode</display-name>
         <type>boolean</type>

.claude/skills/generate-site-preferences-impex/references/attribute-types.md

@@ -18,8 +18,8 @@ Complete reference for all supported SFCC site preference attribute types with e
 ## String
 
 ```xml
-<attribute-definition attribute-id="appApiKey">
-    <display-name xml:lang="x-default">API Key</display-name>
+<attribute-definition attribute-id="appPublicIdentifier">
+    <display-name xml:lang="x-default">Public Identifier</display-name>
     <type>string</type>
     <mandatory-flag>false</mandatory-flag>
     <externally-managed-flag>false</externally-managed-flag>
@@ -127,9 +127,14 @@ Complete reference for all supported SFCC site preference attribute types with e
 
 ## Password (encrypted)
 
+This type is documented for legacy schema compatibility, but masking does not make Site
+preferences an approved secret store. Do not use it for keys, tokens, passwords, credentials,
+or other secret values. Authentication secrets belong in ecom service credentials and
+must be read through `LocalServiceRegistry`.
+
 ```xml
-<attribute-definition attribute-id="appApiSecret">
-    <display-name xml:lang="x-default">API Secret</display-name>
+<attribute-definition attribute-id="appRestrictedNote">
+    <display-name xml:lang="x-default">Restricted Note</display-name>
     <type>password</type>
     <mandatory-flag>false</mandatory-flag>
     <externally-managed-flag>false</externally-managed-flag>

.claude/skills/scaffold-app/SKILL.md

@@ -136,8 +136,8 @@ Generate merchant-facing tasks that guide post-installation setup and verificati
 [
   {
     "name": "Configure API Credentials",
-    "description": "Add your [vendor] API key in Business Manager > Merchant Tools > Custom Site Preferences.",
-    "link": "/on/demandware.store/Sites-Site/default/ViewApplication-BM?SelectedMenuItem=site-prefs_custom_prefs",
+    "description": "Add your [vendor] API credentials in Business Manager > Administration > Operations > Services > Credentials.",
+    "link": "/on/demandware.store/Sites-Site/default/ViewApplication-BM?SelectedMenuItem=service_credentials",
     "taskNumber": "1"
   },
   {
@@ -149,7 +149,7 @@ Generate merchant-facing tasks that guide post-installation setup and verificati
 ```
 
 Tailor tasks to domain for merchant post-installation:
-- **Tax/Shipping/Payment:** Add API credentials, configure service settings, test checkout transactions
+- **Tax/Shipping/Payment:** Add API credentials to ecom service credentials, configure service settings, test checkout transactions
 - **Loyalty/Gift Cards:** Configure points rules, test balance lookups, verify redemption
 - **UI-only apps:** Verify components appear on storefront, check responsive behavior
 - **Fullstack:** Both UI verification and backend configuration steps

.claude/skills/shared/security-rules.md

@@ -2,7 +2,7 @@
 
 Canonical list of all checks performed by `.github/scripts/security-scan.sh`.
 
-## Blocking findings (21 checks — exit code 1)
+## Blocking findings (22 checks — exit code 1)
 
 Must fix before packaging or submission:
 
@@ -19,6 +19,7 @@ Must fix before packaging or submission:
 - S13: setTimeout/setInterval in hook scripts — blocking calls
 - S14: Unbounded loops (while(true)/for(;;)) without break/return
 - S15: Service profiles missing rate-limit-enabled AND circuit-breaker-enabled
+- S20: Secret values stored in Site preferences (any attribute type) or read through `getCustomPreferenceValue(...)`. Secret IDs include API keys, secrets, passwords, credentials, and IDs ending in `Token`. CI flags secret-like `SitePreferences` attribute IDs regardless of declared type and same-line literal secret-ID reads, and fails closed on malformed `SitePreferences` XML. Multiline, computed, or semantically hidden IDs require manual review. **Suggested fix:** remove secret values from Site preferences; store authentication secrets in ecom service credentials and access them through `LocalServiceRegistry` (`svc.configuration.credential.getUser()` / `getPassword()`).
 - P1: Service profile XML missing timeout-millis
 - Q1: Hook scripts referenced in hooks.json that don't exist
 - Q2: Hook scripts missing expected function exports

.github/scripts/security-scan.sh

@@ -50,6 +50,36 @@ strip_comments() {
   grep -vE '^[0-9]+:\s*//' | grep -vE '^[0-9]+:\s*\*' | grep -vE '^[0-9]+:\s*/\*'
 }
 
+# Remove full-line and multiline JavaScript comments while preserving line count.
+# This intentionally stays line-oriented; semantic review handles complex cases.
+sanitize_code() {
+  awk '
+    BEGIN { block = 0 }
+    {
+      if (block) {
+        if ($0 ~ /\*\//) block = 0
+        print ""
+        next
+      }
+      if ($0 ~ /^[[:space:]]*\/\*/) {
+        if ($0 !~ /\*\//) block = 1
+        print ""
+        next
+      }
+      if ($0 ~ /^[[:space:]]*\/\//) {
+        print ""
+        next
+      }
+      print
+    }
+  ' "$1"
+}
+
+if ! command -v xmllint >/dev/null 2>&1; then
+  echo "::error::xmllint is required for Site preference security checks"
+  exit 1
+fi
+
 # Collect JS/DS files (cartridge server-side scripts + storefront-next TS)
 # Using while-read for macOS bash 3 compatibility (no mapfile)
 JS_FILES=()
@@ -169,7 +199,18 @@ for f in ${JS_FILES[@]+"${JS_FILES[@]}"}; do
   [[ -z "$f" ]] && continue
   while IFS= read -r line; do
     block "$f" "Inline Authorization header — use service framework instead: $line"
-  done < <(grep -nE 'setRequestHeader\s*\(\s*['\''"]Authorization' "$f" 2>/dev/null | head -3)
+  done < <(sanitize_code "$f" | awk '
+    {
+      if (statement == "") start = NR
+      statement = statement " " $0
+      if (statement ~ /setRequestHeader[[:space:]]*\([[:space:]]*["\047]Authorization["\047]/) {
+        print start ":" statement
+        statement = ""
+      } else if ($0 ~ /;/) {
+        statement = ""
+      }
+    }
+  ' | head -3)
 done
 
 # S8: Additional DOM sinks — outerHTML, document.write, insertAdjacentHTML (BLOCK)
@@ -321,6 +362,57 @@ for f in ${ALL_CODE_FILES[@]+"${ALL_CODE_FILES[@]}"}; do
   done < <(grep -nE '\bencodeURI\s*\([^)]*(\+|\$\{)' "$f" 2>/dev/null | strip_comments | head -5)
 done
 
+# S20: Secret values stored in or read from Site preferences (BLOCK)
+S20_MESSAGE="Secret value stored in or read from Site preferences — remove it; use ecom service credentials/LocalServiceRegistry for authentication secrets"
+
+s20_secret_site_preference_ids() {
+  # Match secret-like attribute IDs on SitePreferences regardless of <type>.
+  # Password-typed prefs are the obvious case, but string-typed secrets (the
+  # shape the old site-preferences skill generated, e.g. vendorApiKey) are
+  # just as dangerous and BM won't even mask them — so the ID heuristic, not
+  # the declared type, is the gate here.
+  xmllint --xpath '
+    //*[local-name()="type-extension" and @type-id="SitePreferences"]
+      //*[local-name()="attribute-definition"]
+      /@attribute-id
+  ' "$1" 2>/dev/null |
+    grep -oE 'attribute-id="[^"]+"' |
+    cut -d'"' -f2 |
+    awk '{
+      lower = tolower($0)
+      if (lower ~ /(api[_-]?key|secret|password|passwd|credential)/ || lower ~ /token$/) print
+    }'
+}
+
+# Inspect secret-like attribute IDs within the SitePreferences type extension.
+# A malformed SitePreferences XML must fail closed: xmllint cannot parse it, so
+# the xpath above would silently return nothing and a file carrying secret prefs
+# would slip through. Block any SitePreferences-declaring XML that is not
+# well-formed rather than skipping it.
+for f in ${XML_FILES[@]+"${XML_FILES[@]}"}; do
+  [[ -z "$f" ]] && continue
+  if grep -q 'SitePreferences' "$f" 2>/dev/null && ! xmllint --noout "$f" 2>/dev/null; then
+    block "$f" "$S20_MESSAGE: malformed SitePreferences XML could not be parsed — failing closed"
+    continue
+  fi
+  while IFS= read -r preference_id; do
+    block "$f" "$S20_MESSAGE: $preference_id"
+  done < <(s20_secret_site_preference_ids "$f" | head -5)
+done
+
+# Detect literal reads of secret Site preference IDs.
+s20_secret_site_preference_reads() {
+  sanitize_code "$1" |
+    grep -niE "getCustomPreferenceValue[[:space:]]*\\([[:space:]]*['\"]([^'\"]*(api[_-]?key|secret|password|passwd|credential)[^'\"]*|[^'\"]*token)['\"]"
+}
+
+for f in ${JS_FILES[@]+"${JS_FILES[@]}"}; do
+  [[ -z "$f" ]] && continue
+  while IFS= read -r line; do
+    block "$f" "$S20_MESSAGE: $line"
+  done < <(s20_secret_site_preference_reads "$f" | head -5)
+done
+
 echo ""
 
 # ============================ PERFORMANCE ===================================