Refs #45216
The object counterpart to `prefer-coerce-array`: `x ?? {}` becomes
`coerceObject(x)`. The rule flags a `??` whose default is a bare `{}`
literal, so a populated default or an asserted `({} as Foo)` still
passes; a `x ?? y ?? {}` chain maps onto `coerceObject(x, y)`.
Every hit across `lib/` converted cleanly, so no disable comments were
needed.
* test: replace manual process.env mutation with vi.stubEnv
Enable Vitest's `unstubEnvs` so env stubs are reverted automatically
before every test, and convert the manual `process.env.X = ...` /
`delete process.env.X` pairs in specs to `vi.stubEnv()`.
This removes the hand-written `afterEach` cleanup blocks and the
`const OLD_ENV = process.env` / `process.env = { ...OLD_ENV }`
save-restore dance, which silently leaked state between tests when a
test set a variable without a matching delete.
Left as-is (incompatible with `unstubEnvs`, which unstubs before each
test, not after):
- module top-level env writes (cocoapods, gomod, post-update, pip-compile
and bundler artifacts specs)
- `beforeAll` env writes -- the two in `util/http` moved to `beforeEach`
* test: stop asserting on ambient environment variables
Three specs asserted on values that the surrounding environment can
override, so they only passed on a machine that happened not to set them:
- `util/exec/env`: `getChildProcessEnv()` forwards every entry of
`basicEnvVars`, but the spec only seeded a subset. Any other forwarded
variable that was set ambiently (e.g. a lowercase `http_proxy`) leaked
into the `toEqual` assertion. Clear all of them first.
- `util/git/index`: `process.env.GIT_SSH_COMMAND` deliberately overrides
the default that the two env-inheritance tests assert on. Clear it.
- `logger/pretty-stdout`: the colorized level strings are built at import
time, so the `beforeEach` setting `FORCE_COLOR` could never affect them
-- it was dead code, and the test silently assumed a terminal without
colour support. Derive the expectation the same way the module does.
* test(datasource/nuget): use a throwaway cache dir for nupkg downloads
The `determine source URL from nupkg` tests do not mock the filesystem,
so they really wrote downloaded packages into a fixed `/tmp/cache` path.
That fails wherever the system tmpdir is not writable, and two checkouts
running the suite at once would share the same directory.
Use `tmp.dir({ unsafeCleanup: true })` and clean it up afterwards, which
is the pattern the other specs that touch disk already use.
* feat(lint): add renovate/prefer-stub-env rule
Enforces in specs what the migration to `vi.stubEnv` established:
- assigning to or deleting `process.env.X` inside `beforeEach`, `afterEach`,
`it` or `test` is reported, with a fixer that rewrites it to `vi.stubEnv()`
- module scope, `describe` bodies and `beforeAll` / `afterAll` are left alone:
`unstubEnvs` reverts stubs *before* each test, so a stub created there is
gone by the time the first test runs, and those must keep assigning directly
- replacing the `process.env` object is reported everywhere and never fixed.
`vi.stubEnv()` captures the original object when the worker starts, so after
a reassignment its deletes silently target the old object
The rule found 28 further violations in specs the earlier sweep missed, since
that one was anchored on `delete process.env`. Twelve were auto-fixed; the
rest replaced `process.env` wholesale and are now `clearEnv()` from test/util.
`util/exec/index.spec.ts` opts out with a comment: it asserts on the whole
child env, and `vi.stubEnv()` cannot express that because it maps `PROD`,
`DEV` and `SSR` onto '' instead of deleting them.
As noted in #43761, we had recently set `CI=true` for the Renovate and
Containerbase orgs on Mend-hosted apps due to a pnpm v11 requirement:
ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY Aborted removal of
modules directory due to no TTY
If you are running pnpm in CI, set the CI environment variable to
"true", or set "confirmModulesPurge" to "false".
Similarly in #43606, it was noted that we should do this for `mise lock`
to mark the workspace as "trusted".
Although we went with a different situation, there will be cases where
this is useful for tools being executed, for instance when we invoke `yarn`.
We also make sure to keep our documentation up-to-date as we add to the
specific overrides.
With help from Claude Sonnet and Opus to:
- perform some of the mechanical updates as part of this change
- ensure that the Docker sidecar always has `hardcodedProcessEnv` passed
into it
- catch a needed change to `exposeAllEnv` handling
Co-authored-by: Claude Opus 5 <jamie.tanna+claude-code@mend.io>
Co-authored-by: Claude Sonnet 5 <jamie.tanna+claude-code@mend.io>
When executing commands for `binarySource=docker`, we have two areas
that a command could have shell expansion/injection:
- on the host (when constructing the `bash -l -c`)
- in the container (when running a manager-specific command)
When constructing the host's command, we previously would escape the
`"`, which allows Bash syntax for `${VAR}` and `$(...)` syntax inside
it.
While managers use `quote()` to make the inputs safer, the use of `"`
around that would not provide any protection, and lead to cases where
the `quote()` could be expanded on the host, before `docker run`
executes.
After recent changes to each manager - and removal of `shell: true` -
this doesn't seem to be exploitable, but this hardens it further.
Additionally, we never escaped `\`, so a value containing `\"` produces
a command that `shlex.split` rejects, and the update fails rather than
running.
Instead, we can quote the whole `-c` argument with `quote()`, which
round-trips through both `shlex.split` - the default, no-shell path - and
a real shell.
This removes the `lgtm [js/incomplete-sanitization]` suppression as
we're now correctly sanitising this.
Note this doesn't remove the need for the managers to quote their own
values, as the second area above is still a real shell.
Co-authored-by: Claude Opus 5 <jamie.tanna+claude-code@mend.io>
* ci(linter): add prefer-partial-in-specs oxlint rule
Flags object literals cast to `any` (`{...} as any`) in spec/test files and steers toward `partial<T>()` from `test/util`. Enabled at error in the spec-files override so CI blocks new occurrences.
Closes#44646
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test: drop unnecessary object-literal `as any` casts
The casts masked values already assignable to the expected type (migration configs accepted as Record<string, unknown>, all-optional param types, etc.); removing them restores real type checking.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test: replace object-literal `as any` casts with partial<T>()
Uses partial<T>() from test/util so incomplete mock/config objects keep proper typing. Azure API mocks switch to mockResolvedValue(partial<T>()) and their inner method mocks to mockResolvedValue(), matching the sibling-file idiom.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test: suppress prefer-partial-in-specs for intentionally-invalid inputs
Some tests deliberately pass malformed or legacy data to exercise validation/error paths, where no real type fits; keep the cast with a reasoned oxlint-disable-next-line.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* test(util/exec): cover `Executing command` log line
As an important log line, we should make sure it doesn't change without
a test failing.
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* chore(util/exec): log the environment variable names when exerting a command
When debugging, it's often unclear as to whether an environment variable
was passed to a command, unless we see the command fail (which includes
the environment variable names).
We can make sure we log the environment variable names themselves, when
executing the command, to make it more clear.
* fixup! chore(util/exec): log the environment variable names when exerting a command
---------
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* fix(config/global): return default values for `GlobalConfig.get`
As noted in #39949, it can be awkward working with in `GlobalConfig.get`, where
we would previously need to specify the default value when calling `.get`,
which could lead to drift with the defaults for a given config option.
To reduce this risk, we can rely on a given `default` for a configuration
option, providing that as a default.
Because this now definitely returns a value that matches the config option, it
now allows us to remove `!` assertions.
* fixup! fix(config/global): return default values for `GlobalConfig.get`
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* fixup! fix(config/global): return default values for `GlobalConfig.get`
* test(presets): remove impossible test
As it's now not possible to set a value as `undefined` (as
`GlobalConfig` will then use the default), this test no longer validates
anything.
* test(workers/repository): add mock
Now `platform` defaults to `github`, we need to mock out the call to
fetch config.
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* test: coverage
* test: coverage
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <jamie.tanna+claude-code@mend.io>
* feat(managers/npm): allow specifying `toolSettings.nodeMaxMemory`
As noted in #40942, we're seeing an increase in memory usage across
`pnpm` and `yarn` users in Mend-hosted apps.
To allow limiting the maximum memory that any `node` child processes
will use, we can add a new `toolSettings.nodeMaxMemory` to (hopefully)
mitigate this.
Similar to the `toolSettings.jvmMaxMemory`, the self-hosted
administrator can set a maximum, which cannot be exceeded by repo-level
configuration.
* fixup! feat(managers/npm): allow specifying `toolSettings.nodeMaxMemory`
To prefactor this to be a simpler change in the future as we add new
functionality to `getToolSettingsOptions`, we can:
- wrap the JVM settings in its own `describe` block
- use `toMatchObject`, so new default values don't break existing tests
This change introduces a heap size limitation for the Gradle daemon used
when updating the Gradle wrapper, defaults to 512m.
The heap size is limited to the globally configured limit, which
defaults to 512m. It can be configured on the repository level, but not
exceed the globally configured limit. A hard coded mimiumum heap size of
512m is enforced.
The configuration option is named `toolSettings`, which has two
properties for the maximum and initial heap size. See
`docs/usage/configuration-options#toolSettings` for more details.
As part of future changes which will fix issues with Composer + Yarn 1
introduced by f430552de4, we need to find
a way to ignore command failure, on a per-command basis.
To do this, we can provide a refactor to introduce a new type,
`CommandWithOptions` which has the ability to specify whether to
`ignoreFailure`.
To make this less of a refactor than would otherwise require, we can
pass around a `(string | CommandWithOptions)[]`
(or a `string[]` or a `CommandWithOptions[]`) to be used, and use
helpers `asRawCommand` and `asRawCommands` to extract the underlying
command where necessary.
In the case a command's failure is being ignored, return the error code
in the `ExecResult`.
We also need to make sure that when executing in Docker, we still
provide the `|| true`s, otherwise we'll start to see commands failing
unexpectedly.
As part of future-proofing the `CommandWithOptions`, we can make sure
that it uses `string[]`s for the command it's representing, to avoid
re-parsing it.
As noted in #40242, without `shell` being passed around, this won't
correctly apply for Post Upgrade Tasks, and might break other usages.
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
* fix(util/exec): migrate to `execa` for command execution
As part of a future change, we need to make sure that we have
cross-platform support for specific command executions.
By using `execa`'s functionality on top of `node:child_process`, we can
provide this functionality, as well as retain a consistent API with
internal users.
We need to make a couple of small tweaks to setting the `RawExecOptions`
as we're now bound by `readonly` properties.
As the `stdio` property's type is not exported, we can replace this with
an explicit setting of `stdin`, `stdout` and `stderr`.
This requires a number of test changes to make sure that the `stdin`,
`stdout` and `stderr` are set.
This doesn't migrate any calls under `tools`, as they can remain using
`node:child_process`.
* chore(util/exec): ignore floating promises `.on` calls
Since moving to use `execa`, we now have "floating promises" for the
`.on` method calls.
As we aren't handling them here, we can now mark them as `void` (as an
alternative to ignoring the ESLint rule).
This has been deprecated since 2022-07, and is unused as we only ever
use UTF-8.
In `tools/utils/exec.ts` we make sure to specify it explicitly, so we
don't get a Buffer returned.
Sidecar containers will now use `containerbase/` prefix instead of `renovate/` prefix.
BREAKING CHANGE: containerbase/ account used for sidecar containers instead of renovate/
renovate[bot]