Refactor/simplify and secure#34
Merged
Merged
Conversation
|
❌ Tests completed on Node.js 20.x: failure |
- 17 -> 3 vulnerabilities (lint-staged transitive, accepted) - Major-bump @typescript-eslint/* and related dev deps (breaking audit fix) - no-unused-vars: caughtErrors: 'none' to accept the stricter eslint v8 default
Delete 10 src files (~3,000 LOC) and 15 test files (~8,500 LOC) that existed only to support `somon serve`, a management HTTP server with no identifiable user story outside test fixtures. Rationale: the transpiler is a CLI that runs and exits; circuit breakers, Prometheus metrics, a K8s readiness probe, resource limiters, and a structured logger were cargo from an aspirational 'production-grade' framing. Removed: - src/module-system: circuit-breaker, metrics, prometheus-metrics, resource-limiter, structured-logger, runtime-config (ManagementServer), signal-handler - src/cli/serve.ts + the 'serve' subcommand - src/production-validator.ts + --production path through compile/run/bundle Replaced: - src/module-system/logger.ts -> minimal 50-line Logger; stderr for warn/error, SOMON_DEBUG env for info/debug Eliminates two security findings by construction (no ManagementServer => no unauthenticated /config endpoint; no structured logger => no log injection surface). Also removes the Jest mock coercion in stopWatching and the @ts-expect-error reach into CircuitBreaker private fields. BREAKING CHANGE: somon.config.json fields 'metrics', 'circuitBreakers', 'logger', 'managementServer', 'managementPort', 'resourceLimits', 'operationTimeout' are removed. The 'somon serve' subcommand is gone. ModuleLoader constructor no longer accepts metrics/circuitBreakers parameters. Tests: 845 passing / 16 skipped, 0 failures.
…lution 1) Bundler require-rewriter emitted keys via ASCII-only sanitizer (stripped only `'`, `"`, backtick, backslash) and hand-rolled string interpolation. Filenames containing newlines, `)`, `; `, or U+2028/2029 could escape the quoted string and inject arbitrary JS into the bundle. Now emit via `JSON.stringify` which produces a fully-escaped, double- quoted literal regardless of the key shape. 2) ModuleResolver.tryPackageJsonMain resolved `package.json` 'main' without any confinement. A malicious `node_modules/evil/package.json` with "main": "../../../etc/passwd" would trigger an arbitrary-file read when anything imported the package. Enforce that the resolved main path stays inside the package directory. 3) ModuleResolver.resolveMappedPath (user-configured path mappings) now drops targets that escape baseUrl rather than silently following them. 4) Add `isInsideDir` helper for canonical path-containment checks (normalises both sides, compares with trailing sep).
… timeout
The pipeline's documented error model is 'collect diagnostics, never throw
outward'. Four internal throws violated it and could escape the outer
catch in some call chains.
- codegen.generateStatement default case
- codegen.handleUnknownExpression
- codegen.generatePattern default case (even though typed union makes it
unreachable at compile time, a future AST drift would crash here)
- parser.applyCallChaining when '.' is followed by a non-identifier
(reachable via unary -> expressionStatement without the statement-level
try/catch wrapper)
All four now push a diagnostic onto the collector and return a safe
fallback ('' for codegen, the partial expression for the parser).
Also:
- CodeGenerator.compile() setTimeout-based timeout removed. It was a
no-op for synchronous parsing: the throw fired in a future tick,
became an uncaughtException, and did not interrupt CPU-bound work.
If pathological inputs become a problem later, enforce timeouts at
the CLI layer via a worker thread.
- CodeGenerator.mapPropertyName: the 160-entry 'commonMethods' array
was rebuilt on every member expression and scanned with O(n)
Array.includes. Hoisted to a static readonly Set for O(1) lookup.
- CodeGenerator exposes getErrors() so downstream consumers see
codegen diagnostics alongside the existing lexer/parser/type-checker
diagnostics.
Before this change, class method declarations emitted the Tajik name verbatim while call sites ran through mapPropertyName, which could rewrite via the builtins table. Examples 10/11/12/13/17/20/24/27/29/36 (and quick-start-comprehensive) compiled fine but crashed at runtime with 'X is not a function' because the class had, say, `маълумот()` in its body while every caller invoked `info()`. Fix: - generateMethodDefinition now maps the method name through the same builtins table used by mapPropertyName when it is in COMMON_METHODS. - mapPropertyName drops the `looksLikeClassInstance` heuristic (lowercase Cyrillic + underscore). It was the source of the asymmetry: it suppressed call-site mapping while the declaration mapping was unconditional. Symmetric mapping is simpler and consistent — if the user names a method after a builtin, both sides rewrite and the runtime behaves as the user wrote. Result: 70/70 example programs now run. Previously 61/70 worked, 9 were listed as 'partial'.
- cli-integration-real.test.ts no longer calls `npm run build` inside its
`beforeAll`. Under `--coverage` (slower workers), a parallel worker
would nuke `dist/` via `npm run clean` while another worker's
`execSync(node dist/cli.js ...)` was starting — producing up to 21
ENOENT flakes. The suite now expects `dist/` to be built by the caller
and fails fast with a clear message if it is missing.
- performance-regression.test.ts: the whole `describe` was `.skip`ed,
so the '<1000ms per example' claim in CI was relying solely on
scripts/audit-examples.js. Unskipped.
- error-handling.test.ts: the 5 `test.skip`s are split: the one that
actually works after the pipeline changes ('detailed error locations')
is live; the four that need type-checker / error-aggregator features
we do not have yet are back to `.skip` but with precise TODO
comments naming which component would need the work.
- jest-wrapper.sh prefers `node_modules/.bin/jest` and only falls back
to `npx jest` with a warning, preventing silent pulls of jest@30
(which rejects the ts-jest@29 preset) on fresh clones.
Slashmsu
force-pushed
the
refactor/simplify-and-secure
branch
from
24e052b to
2e7d466
Compare
|
❌ Tests completed on Node.js 20.x: failure |
|
❌ Tests completed on Node.js 20.x: failure |
|
❌ Tests completed on Node.js 20.x: failure |
1 similar comment
|
❌ Tests completed on Node.js 20.x: failure |
|
✅ Tests completed on Node.js 20.x: success |
|
✅ Tests completed on Node.js 20.x: success |
|
✅ Tests completed on Node.js 20.x: success |
|
✅ Tests completed on Node.js 20.x: success |
|
✅ Tests completed on Node.js 20.x: success |
|
✅ Tests completed on Node.js 20.x: success |
|
ggulpari
approved these changes
Apr 22, 2026
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
2 participants
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.
🎉 Pull Request
Description
Type of Change
to not work as expected)
Related Issue
Fixes #(issue number)
Changes Made
Testing
npm test)npm run lint)npm run type-check)Checklist
Additional Context
📋 License Information
SomonScript is open source software licensed under the MIT License.
By submitting this pull request, you agree that your contributions will be
licensed under the MIT License.
For contribution guidelines, please review:
guidelines
Thank you for contributing to SomonScript! 🚀