CMS FAQ Management Extension — Source Code Quality Review

Extension: FAQ Management (Epic 193) Source Version Under Review: v1.0.0.57 Vendor: Sataware Technologies LLC Companion Document: CMS FAQ Management — Delivery Review v1.1 (v1.0.0.73) Review Date: March 19, 2026 Prepared by: Phil McCaffrey, ERP Specialist Organization: Bestway (USA) Inc.

---

Executive Summary

Following the Delivery Review (dated March 6, 2026), which evaluated what was delivered against the approved Solution Design, Sataware provided source code on March 19, 2026. The source code is version 1.0.0.57 — 16 revisions behind the v1.0.0.73 compiled binary currently installed in the sandbox. Sataware has not explained the gap or provided the current source.

This document evaluates the quality, security, and maintainability of the delivered source code. The review identified 2 critical security vulnerabilities, 5 high-severity defects, and 18 additional findings across code structure, naming, and standards compliance. Of particular concern: API credentials are hardcoded in plaintext, the Job Queue integration triggers the FAQ API sync twice per scheduled run, and the codeunit ignores the API Setup table entirely — connecting to a hardcoded URL with hardcoded credentials.

This code is not production-ready. The security vulnerabilities alone warrant immediate remediation before any deployment discussion. The structural and quality issues compound the scope gaps identified in the Delivery Review, raising questions about the overall development rigor applied to this engagement.

---

1. Context and Scope

1.1. Source Code Gap

On March 19, 2026, Sataware committed a zip file ("V22 FAQ Management.zip") containing the FAQ Management extension source code. The app.json in the delivered source shows version 1.0.0.57. The compiled .app binary previously delivered (and currently in the sandbox) is version 1.0.0.73.

Artifact	Version	Date
Source code (zip)	1.0.0.57	File dates: Jan 27 – Feb 24, 2026
Compiled .app (sandbox)	1.0.0.73	Mar 6, 2026
Gap	16 revisions	Source code for versions 58–73 not provided

This means the source under review may not match the compiled binary in the sandbox. Any findings based on v1.0.0.57 source may have been addressed — or worsened — in the 16 subsequent revisions. The vendor has not clarified what changed between versions 57 and 73.

1.2. Review Methodology

Every AL source file in the delivered zip was read in full. Each file was evaluated against:

1. 1.2.1. The approved Solution Design (dated 12/16/25), particularly Section 9 ("Data Safety & Integrity")
2. 1.2.2. Bestway's BC extension development standards (documented in the Contribution Guidelines)
3. 1.2.3. Microsoft AL coding best practices and the OWASP Top 10 for application security
4. 1.2.4. The AL anti-pattern catalog maintained in Bestway's code review standards

1.3. File Inventory

The delivered source contains 17 AL files across 8 folders:

Folder	Files	Objects
Codeunit/	1	2 codeunits (65500, 65501) in a single file
Page/	5	5 pages (50001, 50002, 65503, 65504, 65551)
PageExt/	2	4 page extensions (50000, 50001, 50005, 65501) — multiple objects per file
PermissionSet/	1	1 permission set (50100)
Report/	3	2 reports (51000, 65501) + 1 fully commented-out report
ReportLayout/	2	2 RDLC layout files
Table/	3	3 tables (50001, 65503, 65550) + 1 permission set (55101) in same file
TableExt/	2	2 table extensions (50076, 65500) + 1 page extension (65501) + 1 table (50012) — multiple objects per file

---

2. Critical Findings

2.1. Hardcoded API Credentials (CRITICAL — Security)

File: Codeunit/FAQAPIIntegration.Codeunit.al, line 24

The GetAccessToken() procedure connects to the FAQ API with credentials embedded directly in the source code URL:

https://faq.bestwaycorp.com/ExternalAPI/api/Login/token?username=BWUFAQ&password=BWUFAQ123

The credentials are transmitted via HTTP GET request, meaning they will appear in:

1. 2.1.1. Server access logs on faq.bestwaycorp.com
2. 2.1.2. Any proxy, firewall, or WAF logs between BC and the FAQ server
3. 2.1.3. The source code itself (now committed to the repository)
4. 2.1.4. BC telemetry if HTTP diagnostics are enabled

The API Setup table (65503) exists in the extension with fields for Login and URL configuration — but it is never used. The GetAccessToken() procedure calls RecAPISetup.Get() and then ignores the record entirely, proceeding to the hardcoded URL.

Solution Design violation: Section 9 states "Partial updates are avoided." Hardcoded credentials that bypass the configuration table represent a design integrity failure — the table exists to externalize configuration, but the code ignores it.

2.2. Hardcoded Admin Credentials (CRITICAL — Security)

File: Page/AdminPermission.Page.al, lines 35–36

The "Admin Permission" dialog page (used to gate the First Time Sync action) checks user input against hardcoded credentials:

ExpectedUser := 'admin';
ExpectedPass := '12345';

This is not authentication — it is security theater. The password is the most commonly used weak password in existence. The password field is not masked (ExtendedDatatype = Masked is not set). Any user who reads the source code — or guesses "12345" — bypasses the gate. BC's built-in permission system should be used for access control, not a hardcoded dialog.

2.3. Double API Execution on Scheduled Runs (CRITICAL — Data Integrity)

Files: Codeunit/FAQAPIIntegration.Codeunit.al (codeunit 65500) + TableExt/JobQueueLogEntryExt.TableExt.al

The FAQ API is called twice on every scheduled Job Queue run:

1. First call: The Job Queue runs codeunit 65500 (configured via TableNo = "Job Queue Entry"). The OnRun trigger calls GetStandardFormat(Lang), which calls GetAccessToken() and processes the API response.

2. Second call: After the Job Queue completes, it inserts a Job Queue Log Entry. The OnAfterInsert trigger on the log entry table extension (tableextension 50001) checks if the object ID matches codeunit 65500 and, if successful, calls FAQIntegration.GetStandardFormat('en') again.

The second call re-fetches the entire API payload, re-processes all records, and overwrites the counters from the first run. Because most records already exist after the first pass, the insert counts from the second run will be zero — producing misleading email reports.

Solution Design violation: Section 9 states "Repeat executions do not create duplicate or invalid data." While the upsert logic prevents true duplicates, the double execution creates unnecessary API load, doubles database write operations, and corrupts sync counters.

---

3. High-Severity Findings

3.1. Data Loss Risk in Delete-and-Reinsert Pattern (HIGH)

File: Codeunit/FAQAPIIntegration.Codeunit.al, lines 209–214

When updating a Fault/Resolution Relationship record's Service Item Group Code, the code deletes the existing record and re-inserts a copy:

NewFaultResolutionRel := FaultResolutionRel;
FaultResolutionRel.Delete(true);
NewFaultResolutionRel.Validate("Service Item Group Code", ...);
NewFaultResolutionRel.Insert(true);

If the Insert(true) fails — due to a trigger error, permission issue, or constraint violation — the original record has already been permanently deleted. There is no error handling to catch this scenario.

Solution Design violation: Section 6 states "No deletion of records once used in posted transactions." Section 9 states "Partial updates are avoided." The delete-and-reinsert pattern directly violates both principles.

3.2. No Transaction Control in Sync Processing (HIGH)

File: Codeunit/FAQAPIIntegration.Codeunit.al, ProcessStandardFormatResponse procedure

The sync processes potentially hundreds or thousands of records in a single implicit transaction with no error handling. If the HTTP call succeeds but processing fails at record 500 of 1,000, the first 500 records are committed and the remaining 500 are lost. There is no checkpoint, no rollback, and no audit trail of what was processed.

Solution Design violation: Section 9 states "Partial updates are avoided."

3.3. Explicit Commit Breaks Job Queue Error Handling (HIGH)

File: Page/FAQManagement.Page.al, line 1508

The UpdateSyncLogJQ procedure calls Commit() after inserting the sync log record. When called during a Job Queue run, this breaks BC's auto-rollback error handling. If a subsequent operation fails (e.g., email send), the sync log records a successful sync that did not actually complete.

3.4. Report 65501 Modifies Records on Uninitialized Variable (HIGH)

File: Report/FaultResolCodRelationship.Report.al, lines 16–27

The "First Time Sync" report iterates all Fault/Resolution Relationship records and sets each Status to "false." When it looks up the corresponding Fault Area, if the FindFirst() call fails (no matching record), the code falls through to the else branch and calls faultarea.Modify(true) on an uninitialized record variable — which will produce a runtime error.

3.5. Email Recipient Deletion is Destructive (HIGH)

File: Page/EmailRecipientLines.Page.al, lines 33–37

When the email recipient field is cleared, the code calls EmailSetup.DeleteAll() with no filters applied. This deletes all email recipient records for the entire company, regardless of context. In a multi-user environment, one user clearing their email field wipes the recipient list for everyone.

---

4. Structural and Quality Findings

4.1. Multi-Object Files

The AL convention (and Bestway's documented standard) is one object per file. The delivered code has 4 files containing multiple objects:

File	Objects Contained
FAQAPIIntegration.Codeunit.al	Codeunit 65500 + Codeunit 65501
Faultcoderelationext.TableExt.al	TableExtension 65500 + PageExtension 65501 + Table 50012
FaultAreasExt.PageExt.al	PageExtension 50001 + TableExtension 50076
APISetup.Table.al	Table 65503 + PermissionSet 55101

This makes the codebase harder to navigate, review, and maintain. Object discovery requires reading every file rather than relying on folder structure and naming.

4.2. Commented-Out Code (834+ Lines)

File: Page/FAQManagement.Page.al, lines 1–834

The file begins with 834 lines of commented-out code — the entire previous version of the FAQ Management page. This includes dead action handlers, duplicate email body implementations, debugging artifacts, and unreferenced variable declarations. The active code starts at line 838.

File: Report/AttachmentPurpose.Report.al — the entire file (119 lines) is commented out.

Combined, over 950 lines of the delivered source are dead code that should have been removed before delivery.

4.3. Duplicate Procedures

Procedure	Duplicate	Location
insertcuont() (typo)	CountInsert()	Codeunit 65500, lines 237 and 283
ModifyCount()	CountModify()	Codeunit 65500, lines 243 and 288
UpdateStatistics()	UpdateStatisticsJQ()	Page 65504 — nearly identical logic, different variable targets

4.4. Dead Code and Unused Variables

Item	Location	Issue
Beforemodifycount, Aftermodifycount	Codeunit 65500	Cleared but never read
GlobalModifyTrueFalse	Codeunit 65501	Declared but never used
StartTime2, StartDateTime	Page 65504	Written/declared but never read
StartDate	Page 65504	Only used in BuildStartDateTime(), which is never called
BuildStartDateTime(), CalcNextWeekday()	Page 65504	Declared but never called
FMTF()	Page 65504	Declared but never called
OnAfterGetRecord trigger	Page 65504	Page has no SourceTable — trigger never fires
SendEmailAfterSync	Page 65504	Set to true in OnOpenPage, never read
ActiveCountJQ, InactiveCountJQ	Page 65504	Computed in UpdateStatisticsJQ() but never displayed

4.5. Naming Convention Violations

The extension uses no consistent naming prefix. Bestway's standard is {Prefix}{ObjectName}.{ObjectType}.al. Sataware's other extensions use a publisher prefix. This extension uses none.

Issue	Examples
No object prefix	FAQAPIIntegration, AdminPermission, Faultcoderelationext
Typos in procedure names	insertcuont() (should be InsertCount)
Typos in captions	"Genral" (APISetup page), "Status Updaion" (Report 65501 caption)
Cryptic procedure names	FQI(), FQMI(), FMTF() — no indication of purpose
Inconsistent casing	faultarea.reset;, pag.run;, frcr.SetRange(...) mixed with proper casing

4.6. Hardcoded Text Strings

The extension uses hardcoded English strings throughout instead of AL Label variables. This violates BC's localization framework and makes the extension untranslatable.

Examples: 'Unable to connect to FAQ API', 'Login failed. Status: %1', 'Invalid JSON response', 'Token not found', 'API' (Source field value), all email body HTML strings, and all Message() strings.

4.7. DataClassification = ToBeClassified

Three tables/table extensions use DataClassification = ToBeClassified:

1. Table 65503 "API Setup" — contains API login credentials
2. TableExtension 65500 on Fault/Resol. Relationship — Status and Source fields
3. Table 50012 "FAQ Sync Log" — all fields

ToBeClassified is a placeholder that should be replaced with the appropriate classification (e.g., CustomerContent, SystemMetadata, EndUserIdentifiableInformation) before production deployment.

4.8. ID Range Overlap Risk

The app.json declares a single ID range of 50000–999999. This is excessively broad and overlaps with every other extension in the repository. Several objects use IDs in the 50000–50100 range, which overlaps with BestwayUSA's reserved range (50100–50149):

Object	ID	Risk
Table ServiceItemGroupCodeBuffer	50001	Potential collision
Table "FAQ Sync Log"	50012	Potential collision
Page AdminPermission	50001	Potential collision
Page ServiceItemGroupCodeBufferPage	50002	Potential collision
PermissionSet SatawareExtPermiss	50100	Overlaps BestwayUSA range
Multiple PageExtensions	50000, 50001, 50005	Potential collision
TableExtension FaultAreaExt	50076	Potential collision

4.9. Missing Permission Set Coverage

The extension defines two permission sets:

• SatawareExtPermiss (50100): Grants RIMD on ServiceItemGroupCodeBuffer and Email Recipient Setup
• API Setup Perm (55101): Grants RIMD on API Setup

Neither permission set grants access to FAQ Sync Log (table 50012). Users who need to view sync history will get a permission error.

4.10. Missing Telemetry

The app.json does not include an applicationInsightsConnectionString. Without extension-level telemetry, Bestway cannot monitor sync failures, API errors, or performance issues in production.

4.11. Missing Namespace Declaration

The extension targets application: "27.0.0.0" but declares no namespace. Post-BC 25.0 extensions should use namespace declarations for proper scoping.

4.12. Email Body HTML Colspan Mismatch

File: Page/FAQManagement.Page.al, line 1471

The JQ email body table header defines 11 columns, but the "no records" fallback row uses colspan="8". This causes the fallback message to render incorrectly, leaving 3 columns unspanned.

---

5. Solution Design Compliance — Section 9 Violations

The Solution Design's Section 9 ("Data Safety & Integrity") makes four specific commitments. The delivered code violates three:

Commitment	Status	Evidence
No modification of posted documents	Appears compliant	No code touches posted documents
Inactive handling ensures backward compatibility	Partially compliant	Status fields exist but only on 2 of 6 tables (per Delivery Review)
Partial updates are avoided	VIOLATED	No transaction control in sync processing (Section 3.2); Delete-and-reinsert can leave orphaned deletions (Section 3.1)
Repeat executions do not create duplicate or invalid data	VIOLATED	Double API execution on Job Queue runs (Section 2.3); Sync counters are corrupted by the second pass

---

6. Summary

6.1. Finding Summary by Severity

Severity	Count	Key Items
CRITICAL	3	Hardcoded API credentials, hardcoded admin password, double API execution
HIGH	5	Data loss risk (delete-reinsert), no transaction control, explicit Commit breaks rollback, uninitialized record Modify, destructive DeleteAll
MEDIUM	8	Multi-object files, 950+ lines commented-out code, duplicate procedures, dead code, naming violations, hardcoded strings, DataClassification placeholders, ID range overlap
LOW	5	Missing telemetry, missing namespace, colspan mismatch, missing permission coverage, missing inline documentation

6.2. Combined Assessment (Delivery Review + Code Quality Review)

The Delivery Review (v1.1) found that 1 of 20 required fields was implemented and 4 of 6 deliverables were either not delivered or unverifiable. This Code Quality Review finds that the code that was delivered contains critical security vulnerabilities, violates the vendor's own Solution Design commitments, and does not meet basic professional development standards.

Taken together, these documents establish that the delivered extension:

1. 6.2.1. Does not implement the scope defined in the approved Solution Design
2. 6.2.2. Contains security vulnerabilities that would be flagged in any standard code review
3. 6.2.3. Violates three of four data safety commitments made in the Solution Design
4. 6.2.4. Was delivered without source code matching the deployed version (16-revision gap)
5. 6.2.5. Was delivered without test plans, test codeunits, technical documentation, or telemetry

---

7. Recommendations

7.1. Immediate Actions

1. 7.1.1. Do not deploy v1.0.0.57 or v1.0.0.73 to production. The security vulnerabilities and data integrity risks are unacceptable.
2. 7.1.2. Request the v1.0.0.73 source code. The delivered v1.0.0.57 source does not match the deployed binary. Any remediation discussion must be based on the current source.
3. 7.1.3. Revoke the hardcoded FAQ API credentials (BWUFAQ / BWUFAQ123) and issue new credentials stored securely (either in the API Setup table or via BC's Isolated Storage).

7.2. Before Any Future Deployment

1. 7.2.1. All hardcoded credentials must be removed and replaced with configurable, secure storage
2. 7.2.2. The double API execution in the Job Queue trigger must be eliminated
3. 7.2.3. The delete-and-reinsert pattern must be replaced with a safe Modify operation
4. 7.2.4. Transaction control must be added to the sync processing loop
5. 7.2.5. The explicit Commit() must be removed or moved outside the sync transaction boundary
6. 7.2.6. All commented-out code must be removed
7. 7.2.7. DataClassification must be properly assigned on all fields
8. 7.2.8. Extension-level telemetry must be configured
9. 7.2.9. ID ranges must be narrowed and verified against all other extensions in the repository

7.3. Discussion Points for Sataware

1. 7.3.1. Why are API credentials hardcoded when the API Setup table exists for this purpose?
2. 7.3.2. Was the double API execution on Job Queue runs known? Was it tested?
3. 7.3.3. Why was 950+ lines of commented-out code included in a production delivery?
4. 7.3.4. What is the source code for versions 58–73? What changed?
5. 7.3.5. Was any code review or quality assurance process applied before delivery?

---

Appendix A: Object Inventory

Object Type	ID	Name	File
Codeunit	65500	FAQ API Integration	Codeunit/FAQAPIIntegration.Codeunit.al
Codeunit	65501	FAQ Sync Counter Manager	Codeunit/FAQAPIIntegration.Codeunit.al
Page	50001	AdminPermission	Page/AdminPermission.Page.al
Page	50002	ServiceItemGroupCodeBufferPage	Page/ServiceItemGroupCodeBufferPage.Page.al
Page	65503	API Setup	Page/APISetup.Page.al
Page	65504	FAQ Management	Page/FAQManagement.Page.al
Page	65551	Email Recipient Lines	Page/EmailRecipientLines.Page.al
PageExtension	50000	ServiceSubformFilterExt	PageExt/ServiceSubformFilterExt.PageExt.al
PageExtension	50001	FaultAreasExt	PageExt/FaultAreasExt.PageExt.al
PageExtension	50005	ServiceItemformFilterExt	PageExt/ServiceSubformFilterExt.PageExt.al
PageExtension	65501	Faultcoderelationextpage	TableExt/Faultcoderelationext.TableExt.al
PermissionSet	50100	SatawareExtPermiss	PermissionSet/SatawareExtPermiss.PermissionSet.al
PermissionSet	55101	API Setup Perm	Table/APISetup.Table.al
Report	51000	FaultResolCodRelationshiprep	Report/FaultResolCodRelationshiprep.Report.al
Report	65501	Fault/ResolCodRelationship	Report/FaultResolCodRelationship.Report.al
Table	50001	ServiceItemGroupCodeBuffer	Table/ServiceItemGroupCodeBuffer.Table.al
Table	50012	FAQ Sync Log	TableExt/Faultcoderelationext.TableExt.al
Table	65503	API Setup	Table/APISetup.Table.al
Table	65550	Email Recipient Setup	Table/EmailRecipientSetup.Table.al
TableExtension	50076	FaultAreaExt	PageExt/FaultAreasExt.PageExt.al
TableExtension	65500	Faultcoderelationext	TableExt/Faultcoderelationext.TableExt.al

Appendix B: Reference Documents

Document	Date	Relationship
CMS FAQ Management — Delivery Review v1.1	Mar 6, 2026	Companion: evaluates scope delivery against Solution Design
Solution Design (v1)	Dec 16, 2025	Approved specification for the extension
This Code Quality Review	Mar 19, 2026	Evaluates code quality of delivered source