feat: add Asset Scan Status integration tests to sanity suite#567
Open
aniket-shikhare-cstk wants to merge 9 commits into
Open
feat: add Asset Scan Status integration tests to sanity suite#567aniket-shikhare-cstk wants to merge 9 commits into
aniket-shikhare-cstk wants to merge 9 commits into
Conversation
Add 3 integration tests under a new 'Asset Scan Status' describe block in asset-test.js: - Single fetch with include_asset_scan_status=true: validates the _asset_scan_status field (pending|clean|quarantined|not_scanned) when present; skips assertion when feature is not enabled for the stack. - List query with include_asset_scan_status=true: same opt-in validation on the first item returned. - Fetch without param: asserts _asset_scan_status is absent from response. Reuses the image asset uploaded by the Asset Upload block via testData.assets.image.uid to avoid extra uploads. Falls back to creating a fresh asset only if the upload block did not run. Related branch: feat/DX-8752-asset-scan
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
|
Coverage report for commit: 9150bde Summary - Lines: 82.73% | Methods: 95.87% | Branches: 65.85%
🤖 comment via lucassabreu/comment-coverage-clover |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…org) Add assetScanStatus-test.js (Phase 6.5) covering 12 test cases on the non-AM org stack and 7 test cases on the AM/DAM org stack: Non-AM Org (ORGANIZATION, scan enabled): 1. Freshly uploaded asset returns _asset_scan_status (pending/clean) 2. Status value is a valid enum (pending|clean|quarantined|not_scanned) 3. Status ABSENT from single fetch when param is omitted 4. ALL file items in list carry status when param is passed 5. NO list items carry status when param is omitted 6. Status consistent between single-fetch and list-query for same asset 7. Status present when combined with version=1 param 8. Status present when combined with locale=en-us param 9. Status resets to pending after asset file is replaced 10. Folder entries do NOT get _asset_scan_status 11. Status present across multiple pages of paginated list 12. Status absent when include_asset_scan_status=false AM Org (AM_API_KEY required, DAM + scan enabled): AM-1..7: same coverage for DAM asset upload pipeline Also: - Registers the suite in sanity.js as Phase 6.5 (after asset-test.js) - Adds AM_API_KEY placeholder to .env with setup instructions - Updates .talismanrc for false-positive secret scan patterns
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Based on 'Asset Scanning Support – SDK Design Document'.
New test sections added to assetScanStatus-test.js:
§ 3.2 Upload with param:
- Upload response includes _asset_scan_status=pending when param passed
- Upload response does NOT include status without param
§ 3.3 Download error handling:
- SDK must surface asset_scan_pending/quarantined error codes (not swallow)
- Download 422 errors must expose status + message to callers
§ 3.4 Publish is always async:
- Publish returns success notice regardless of asset scan status
- SDK must never throw asset_scan_quarantined synchronously on publish
§ 3.6 Legacy asset null handling:
- null is a valid _asset_scan_status for pre-scan legacy assets
- SDK must not convert null to undefined or crash on null status
- Updated all list-query assertions to accept null per spec
§ 4.2 api_version header isolation:
- After bulkOperation.publish({ api_version: '3.2' }), subsequent
asset and content-type fetches must NOT carry api_version header
- Regression guard against global header pollution
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
…message chai's have.property(name, val) treats val as expected value, not error message. This caused test 11 (pagination) to fail because chai checked _asset_scan_status === 'Page item blt... is missing...' instead of just verifying the property exists. All 7 affected assertions fixed: - 5x .not.have.property(name, msg) → .not.have.property(name) (false-pass bug: would pass even if property existed with any value) - 1x .have.property(name, msg) → expect(obj, msg).to.have.property(name) (was the direct failure in pagination test) - 1x api_version header check in §4.2 Verified locally: 23 passing, 0 failing, 9 pending (publish/AM skips expected)
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
… AM_API_KEY The AM Org test suite now follows the same pattern as the main sanity suite: - Uses the existing authtoken from testSetup.testContext (already logged in) - Creates a fresh stack inside AM_ORG_UID via POST /v3/stacks at test start - Runs all 7 AM org asset scan tests against the dynamic stack - Deletes the AM stack in the after() hook No static AM_API_KEY needed in .env — removed from environment config. Fallback: skip the AM suite gracefully if AM_ORG_UID is not set. Verified locally: 30 passing, 2 pending (publish tests — expected, fresh stack has no environments; these will run in the full suite)
The § 3.4 publish tests were pending because the fresh dynamic stack has no environments (they're only created by Phase 5 environment tests). Fix: before() now creates a throw-away environment when none is found, and after() deletes it. Same self-contained pattern used by AM org stack. Priority order: 1. Use testData.environments.development.name if Phase 5 already ran 2. Query for any existing environment 3. Create a temporary environment (scan-publish-env-XXXXX) Verified locally: 32 passing, 0 pending, 0 failing
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
…ICAR
Ports the pattern from the Python CMA SDK tests (test_06_asset.py,
test_31_am_assets.py) to JavaScript.
New helpers:
- EICAR_BASE64: standard 68-byte EICAR signature stored base64-encoded
so source file is never flagged by Talisman / repo antivirus scanners
- createEicarFile(): decodes to a temp file at runtime, deleted in after()
- waitForScan(stack, uid, expected, timeout=60s): polls
fetch({include_asset_scan_status:true}) every 3s until status matches
or times out; treats not_scanned as terminal (feature disabled on stack)
New describe: 'Asset Scan Status – Scan Lifecycle (clean + quarantined)'
- clean image → polls until 'clean'
- EICAR file → polls until 'quarantined'
- quarantined download → verifies SDK surfaces 403/422 with scan error code
(§ 3.3 re-tested with a REAL quarantined asset, not a fake URL)
AM org additions (AM-8, AM-9):
- [AM] clean image transitions pending → clean
- [AM] EICAR file reaches quarantined status
Also adds: import fs from 'fs' and import os from 'os'
Verified locally: 37 passing, 0 pending, 0 failing
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
There was a problem hiding this comment.
Pull request overview
This PR extends the sanity (live-stack) test suite around assets by adding new coverage for the include_asset_scan_status query parameter, and also introduces additional retry/backoff behavior to reduce flakiness in sanity runs.
Changes:
- Adds
Asset Scan Statussanity coverage via a newassetScanStatus-test.jssuite and an additionalAsset Scan Statusblock inasset-test.js. - Adds retry behavior to bulk operation job-status sanity tests and introduces a global “wait before retry” delay in the sanity runner.
- Updates
.talismanrcto ignore secret-scanning checks for the modified sanity files (broad file-level allowlist).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
test/sanity-check/sanity.js |
Imports the new scan-status test suite and adds a global retry backoff in beforeEach. |
test/sanity-check/api/bulkOperation-test.js |
Increases job-ready polling and adds retries to several job status assertions. |
test/sanity-check/api/assetScanStatus-test.js |
Adds a large, multi-section integration suite for scan-status behavior (including lifecycle + quarantined/EICAR paths, publish behavior, and AM-org stack creation). |
test/sanity-check/api/asset-test.js |
Adds a smaller “Asset Scan Status” describe block with 3 basic assertions. |
.talismanrc |
Adds file-level Talisman ignore entries for the sanity runner and the new scan test file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+475
to
+478
| // Use an invalid URL to trigger a predictable SDK error and verify propagation | ||
| try { | ||
| await stack.asset().download({ url: 'https://invalid-host.example.com/nonexistent-asset', responseType: 'blob' }) | ||
| } catch (err) { |
Comment on lines
+503
to
+506
| // Attempt download with a deliberately bad URL (simulates scan-blocked response) | ||
| const assetObj = await stack.asset(assetUid).fetch() | ||
| await assetObj.download({ url: assetObj.url + '?__scan_error_test=1', responseType: 'arraybuffer' }) | ||
| } catch (err) { |
Comment on lines
+4
to
+7
| - filename: test/sanity-check/sanity.js | ||
| checksum: f8b4b4b4492e04fd13338af9d99972807b4d61e2b0237a6c057d3f93d8c66d60 | ||
| - filename: test/sanity-check/api/assetScanStatus-test.js | ||
| checksum: e3b1857fbe321e7125b55613acd58c3c0b2fd2462e7a14e48279ad35db4169e7 |
Comment on lines
+84
to
+86
| // Phase 6.5: Asset Scan Status - comprehensive tests for include_asset_scan_status param | ||
| // Covers both ORGANIZATION stack (non-AM, scan enabled) and AM_ORG_UID stack (AM_API_KEY, scan enabled) | ||
| import './api/assetScanStatus-test.js' |
Comment on lines
+1
to
+29
| /** | ||
| * Asset Scan Status - Comprehensive Integration Tests | ||
| * | ||
| * Based on: "Asset Scanning Support – SDK Design Document" | ||
| * | ||
| * Tests the `include_asset_scan_status` API parameter across two org contexts: | ||
| * | ||
| * Part 1 – Non-AM Org (ORGANIZATION, scan plan enabled) | ||
| * Stack is the dynamic stack created by testSetup under process.env.ORGANIZATION. | ||
| * Uses process.env.API_KEY set at runtime. | ||
| * | ||
| * Part 2 – AM Org (AM_ORG_UID, DAM / Contentstack Assets + scan enabled) | ||
| * A stack is created dynamically inside AM_ORG_UID using the same authtoken | ||
| * obtained during main setup. No static AM_API_KEY required. | ||
| * All tests in Part 2 are skipped when AM_ORG_UID is not set. | ||
| * | ||
| * Bug surface these tests cover (per design doc): | ||
| * § 3.1 - Scan status missing/leaking on fetch and list | ||
| * § 3.1 - Param silently dropped when combined with version/locale/pagination | ||
| * § 3.2 - Upload response does NOT include status even when param is passed | ||
| * § 3.3 - SDK swallowing download errors for pending/quarantined assets | ||
| * (error codes: asset_scan_pending / asset_scan_quarantined → 422) | ||
| * § 3.4 - Publish blocking synchronously on scan status (should be async) | ||
| * § 3.5 - Bulk publish blocking synchronously on scan status | ||
| * § 3.6 - Legacy asset null status causing SDK to crash or fail validation | ||
| * § 4.2 - api_version: 3.2 header bleeding into non-publish SDK calls | ||
| * General - Status not reset to 'pending' after file replace | ||
| * General - Folder entries incorrectly receiving a scan status field | ||
| * General - Status inconsistent between single-fetch and list endpoints |
Comment on lines
+545
to
+548
| // Write EICAR test file to a system temp path | ||
| try { eicarFilePath = createEicarFile() } catch (e) { | ||
| console.log(' [scan-test] EICAR file creation failed:', e.message) | ||
| } |
Comment on lines
+989
to
+1039
| describe('Asset Scan Status', () => { | ||
| let scanAssetUid | ||
|
|
||
| before(async function () { | ||
| this.timeout(30000) | ||
| // Reuse the image asset uploaded earlier — avoids a redundant upload | ||
| if (testData.assets && testData.assets.image && testData.assets.image.uid) { | ||
| scanAssetUid = testData.assets.image.uid | ||
| return | ||
| } | ||
| // Fallback: create a fresh asset if the upload block didn't run | ||
| try { | ||
| const asset = await stack.asset().create({ | ||
| upload: assetPath, | ||
| title: `Scan Status Asset ${Date.now()}`, | ||
| description: 'Fallback asset for scan-status tests' | ||
| }) | ||
| scanAssetUid = asset.uid | ||
| } catch (err) { | ||
| // Individual tests will self-skip when scanAssetUid is unset | ||
| } | ||
| }) | ||
|
|
||
| it('should accept include_asset_scan_status param on single asset fetch', async function () { | ||
| this.timeout(15000) | ||
| if (!scanAssetUid) return this.skip() | ||
|
|
||
| const asset = await stack.asset(scanAssetUid).fetch({ include_asset_scan_status: true }) | ||
|
|
||
| expect(asset).to.be.an('object') | ||
| expect(asset.uid).to.equal(scanAssetUid) | ||
| // _asset_scan_status is opt-in: present only when asset scanning is enabled for the stack. | ||
| // Possible values: 'pending' | 'clean' | 'quarantined' | 'not_scanned' | ||
| if ('_asset_scan_status' in asset) { | ||
| expect(asset._asset_scan_status).to.be.a('string') | ||
| expect(['pending', 'clean', 'quarantined', 'not_scanned']).to.include(asset._asset_scan_status) | ||
| } | ||
| }) | ||
|
|
||
| it('should accept include_asset_scan_status param on asset list query', async function () { | ||
| this.timeout(15000) | ||
|
|
||
| const response = await stack.asset().query({ include_asset_scan_status: true }).find() | ||
|
|
||
| expect(response).to.be.an('object') | ||
| expect(response.items).to.be.an('array') | ||
| if (response.items.length > 0 && '_asset_scan_status' in response.items[0]) { | ||
| expect(response.items[0]._asset_scan_status).to.be.a('string') | ||
| expect(['pending', 'clean', 'quarantined', 'not_scanned']).to.include(response.items[0]._asset_scan_status) | ||
| } | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Asset Scan Statusdescribe block totest/sanity-check/api/asset-test.js— the only missing integration test that can be added todevelopmentnow (the other two feature branches need SDK changes merged first).include_asset_scan_statusquery param behaviour.Tests added
include_asset_scan_status=true_asset_scan_statusfield, when present, is one ofpending|clean|quarantined|not_scannedinclude_asset_scan_status=true_asset_scan_statuskey is absent from responseAll three tests are soft on the scan-status field itself (asset scanning may not be enabled on every test stack) but hard on structure and absence-when-not-requested.
Gap analysis (other feature branches)
feat/DX-8752-asset-scanenh/dx-7264.publish()/.unpublish()on Variantsenh/api_version_managementapi_versionviaserviceVersionmoduleTest plan
sdk-js-cma-daily-sanitypipeline —Asset Scan Statusblock should show 3 passing tests (fields may be absent on stacks where scanning is disabled — tests handle that gracefully).🤖 Generated with Claude Code