Skip to content

Fix TypeScript compilation errors in security module #491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hotlong merged 3 commits into from
Feb 2, 2026
Merged

Fix TypeScript compilation errors in security module #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e95455a
Initial plan
Copilot Feb 2, 2026
81f8404
Fix TypeScript compilation errors in packages/core/src/security/
Copilot Feb 2, 2026
f99abfd
Fix remaining TypeScript errors: use issues instead of errors, change…
Copilot Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/core/src/plugin-loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ export interface ServiceRegistration {
}

/**
* Plugin Configuration Validator
* Plugin Configuration Validator Interface
* Uses Zod for runtime validation of plugin configurations
* @deprecated Use the PluginConfigValidator class from security module instead
*/
export interface PluginConfigValidator {
export interface IPluginConfigValidator {
schema: z.ZodSchema;
validate(config: any): any;
}
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/security/plugin-config-validator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { z } from 'zod';
import { PluginConfigValidator } from '../plugin-config-validator.js';
import { createLogger } from '../../logger.js';
import type { PluginMetadata } from '../../plugin-loader.js';
import { PluginConfigValidator } from './plugin-config-validator.js';
import { createLogger } from '../logger.js';
import type { PluginMetadata } from '../plugin-loader.js';

describe('PluginConfigValidator', () => {
let validator: PluginConfigValidator;
let logger: ReturnType<typeof createLogger>;

beforeEach(() => {
logger = createLogger({ level: 'silent' });
logger = createLogger({ level: 'error' });
validator = new PluginConfigValidator(logger);
});

Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/security/plugin-config-validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class PluginConfigValidator {
...formattedErrors.map(e => ` - ${e.path}: ${e.message}`),
].join('\n');

this.logger.error(errorMessage, {
this.logger.error(errorMessage, undefined, {

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

Inconsistent logger.error usage within this file. This call uses the 3-parameter signature with undefined as the second parameter, but other calls in the same file (lines 152, 304, 330) use the 2-parameter signature when there's no Error object. For consistency, when logging without an Error object, use the 2-parameter signature: this.logger.error(errorMessage, { plugin: plugin.name, errors: formattedErrors }) instead of the 3-parameter version with undefined.

Suggested change
this.logger.error(errorMessage, undefined, {
this.logger.error(errorMessage, {

Copilot uses AI. Check for mistakes.
plugin: plugin.name,
errors: formattedErrors,
});
Expand All @@ -89,7 +89,8 @@ export class PluginConfigValidator {

try {
// Use Zod's partial() method for partial validation
const partialSchema = plugin.configSchema.partial();
// Cast to ZodObject to access partial() method
const partialSchema = (plugin.configSchema as any).partial();
const validatedConfig = partialSchema.parse(partialConfig);

this.logger.debug(`✅ Partial config validated: ${plugin.name}`);
Expand Down Expand Up @@ -171,8 +172,8 @@ export class PluginConfigValidator {

// Private methods

private formatZodErrors(error: z.ZodError): Array<{path: string; message: string}> {
return error.errors.map(e => ({
private formatZodErrors(error: z.ZodError<any>): Array<{path: string; message: string}> {
return error.issues.map((e: z.ZodIssue) => ({
path: e.path.join('.') || 'root',
message: e.message,
}));
Expand Down
21 changes: 15 additions & 6 deletions packages/core/src/security/plugin-permission-enforcer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { PluginPermissionEnforcer, SecurePluginContext } from '../plugin-permission-enforcer.js';
import { createLogger } from '../../logger.js';
import { PluginPermissionEnforcer, SecurePluginContext } from './plugin-permission-enforcer.js';
import { createLogger } from '../logger.js';
import type { PluginCapability } from '@objectstack/spec/system';
import type { PluginContext } from '../../types.js';
import type { PluginContext } from '../types.js';

describe('PluginPermissionEnforcer', () => {
let enforcer: PluginPermissionEnforcer;
let logger: ReturnType<typeof createLogger>;

beforeEach(() => {
logger = createLogger({ level: 'silent' });
logger = createLogger({ level: 'error' });
enforcer = new PluginPermissionEnforcer(logger);
});

Expand All @@ -23,6 +23,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -43,6 +44,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -63,6 +65,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -82,6 +85,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -106,6 +110,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -125,6 +130,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -146,6 +152,7 @@ describe('PluginPermissionEnforcer', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand All @@ -165,12 +172,12 @@ describe('SecurePluginContext', () => {
let mockBaseContext: PluginContext;

beforeEach(() => {
logger = createLogger({ level: 'silent' });
logger = createLogger({ level: 'error' });
enforcer = new PluginPermissionEnforcer(logger);

mockBaseContext = {
registerService: () => {},
getService: (name: string) => ({ name }),
getService: <T>(name: string): T => ({ name } as any),
getServices: () => new Map(),
hook: () => {},
trigger: async () => {},
Expand All @@ -189,6 +196,7 @@ describe('SecurePluginContext', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand Down Expand Up @@ -221,6 +229,7 @@ describe('SecurePluginContext', () => {
version: { major: 1, minor: 0, patch: 0 },
},
conformance: 'full',
certified: false,
},
];

Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/security/plugin-signature-verifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { PluginMetadata } from '../plugin-loader.js';

// Conditionally import crypto for Node.js environments
let cryptoModule: typeof import('crypto') | null = null;
if (typeof window === 'undefined') {
if (typeof (globalThis as any).window === 'undefined') {

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

The approach using typeof (globalThis as any).window is a workaround for TypeScript compilation in Node.js environments. A more idiomatic approach would be to check for Node.js using typeof process !== 'undefined' && process.versions?.node !== undefined (as used in logger.ts:27) rather than checking for the absence of window. This is more explicit about detecting the Node.js environment and avoids the type assertion.

Suggested change
if (typeof (globalThis as any).window === 'undefined') {
if (typeof process !== 'undefined' && process.versions?.node !== undefined) {

Copilot uses AI. Check for mistakes.
try {
// Dynamic import for Node.js crypto module
// eslint-disable-next-line @typescript-eslint/no-var-requires
Expand Down Expand Up @@ -132,7 +132,7 @@ export class PluginSignatureVerifier {

if (!isValid) {
const error = `Signature verification failed for plugin: ${plugin.name}`;
this.logger.error(error, { plugin: plugin.name, publisherId });
this.logger.error(error, undefined, { plugin: plugin.name, publisherId });

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

Inconsistent logger.error usage within this file. This call uses the 3-parameter signature with undefined as the second parameter, but other calls in the same file (lines 152, 304, 330) use the 2-parameter signature when there's no Error object. For consistency, when logging without an Error object, use the 2-parameter signature: this.logger.error(error, { plugin: plugin.name, publisherId }) instead of this.logger.error(error, undefined, { plugin: plugin.name, publisherId }).

Suggested change
this.logger.error(error, undefined, { plugin: plugin.name, publisherId });
this.logger.error(error, { plugin: plugin.name, publisherId });

Copilot uses AI. Check for mistakes.
throw new Error(error);
}

Expand Down Expand Up @@ -190,7 +190,7 @@ export class PluginSignatureVerifier {
private handleUnsignedPlugin(plugin: PluginMetadata): SignatureVerificationResult {
if (this.config.strictMode) {
const error = `Plugin missing signature (strict mode): ${plugin.name}`;
this.logger.error(error, { plugin: plugin.name });
this.logger.error(error, undefined, { plugin: plugin.name });

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

Inconsistent logger.error usage within this file. This call uses the 3-parameter signature with undefined as the second parameter, but other calls in the same file (lines 152, 304, 330) use the 2-parameter signature when there's no Error object. For consistency, when logging without an Error object, use the 2-parameter signature: this.logger.error(error, { plugin: plugin.name }) instead of this.logger.error(error, undefined, { plugin: plugin.name }).

Suggested change
this.logger.error(error, undefined, { plugin: plugin.name });
this.logger.error(error, { plugin: plugin.name });

Copilot uses AI. Check for mistakes.
throw new Error(error);
}

Expand Down Expand Up @@ -220,7 +220,7 @@ export class PluginSignatureVerifier {

private computePluginHash(plugin: PluginMetadata): string {
// In browser environment, use SubtleCrypto
if (typeof window !== 'undefined') {
if (typeof (globalThis as any).window !== 'undefined') {

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

The approach using typeof (globalThis as any).window is inconsistent with the codebase pattern for environment detection. A more idiomatic approach would be to check for Node.js using typeof process !== 'undefined' && process.versions?.node !== undefined (as used in logger.ts:27) rather than checking for the absence of window. This is more explicit about detecting the Node.js environment and avoids the type assertion.

Copilot uses AI. Check for mistakes.
return this.computePluginHashBrowser(plugin);
}

Expand Down Expand Up @@ -287,7 +287,7 @@ export class PluginSignatureVerifier {
publicKey: string
): Promise<boolean> {
// In browser environment, use SubtleCrypto
if (typeof window !== 'undefined') {
if (typeof (globalThis as any).window !== 'undefined') {

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

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

The approach using typeof (globalThis as any).window is inconsistent with the codebase pattern for environment detection. A more idiomatic approach would be to check for Node.js using typeof process !== 'undefined' && process.versions?.node !== undefined (as used in logger.ts:27) rather than checking for the absence of window. This is more explicit about detecting the Node.js environment and avoids the type assertion.

Copilot uses AI. Check for mistakes.
return this.verifyCryptoSignatureBrowser(data, signature, publicKey);
}

Expand Down
Loading