Skip to content

Commit d3f5c8b

Browse files
jonathanhefnerjonathanhefner
authored
Merge pull request #35 from agentplugins/agent/environment-precedence
Check configured environment precedence
2 parents 6409e11 + fce2ba0 commit d3f5c8b

10 files changed

Lines changed: 204 additions & 29 deletions

File tree

‎plugins/agent-plugins-conformance-core/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ Replace the path with a downloaded or installed copy of this plugin; the command
3131
| `mcp.stdio.env.plugin-data-absolute` | Supplied `PLUGIN_DATA` is an absolute path. This check does not exercise writability, storage dedication, or persistence across updates. |
3232
| `filesystem.data.writable` | The default server can create, write, and close a small temporary file directly in the supplied `PLUGIN_DATA` directory. |
3333
| `filesystem.data.consistent-within-plugin` | Servers using omitted and data-rooted working directories resolve `PLUGIN_DATA` to the same filesystem path. |
34-
| `mcp.stdio.env.configured-value` | The configured value reaches the subprocess. The fixture does not arrange a conflicting inherited value, so it does not establish override behavior. |
34+
| `mcp.stdio.env.configured-value` | The configured value reaches the subprocess. |
35+
| `mcp.stdio.env.configured-precedence` | Configured environment values replace conflicting base-environment values. |
3536
| `mcp.stdio.args.preservation-and-expansion` | Argument boundaries, including spaces and an empty argument, are preserved; recognized placeholders expand and unknown placeholder-like text stays literal. |
3637
| `mcp.stdio.command.single-token` | A command path containing a space is preserved as a single executable token. |
3738
| `mcp.stdio.env.expansion` | Repeated recognized placeholders expand in environment values and unknown placeholder-like text stays literal. |

‎plugins/agent-plugins-conformance-core/dist/probe.mjs‎

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎plugins/agent-plugins-conformance-core/mcp.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
"env": {
1919
"APC_VALUE": "fixture value with spaces",
2020
"APC_EXPANSION": "${PLUGIN_ROOT}|${PLUGIN_DATA}|${PLUGIN_ROOT}",
21-
"APC_LITERAL": "${APC_UNKNOWN}|$APC_VALUE|${PLUGIN_ROOT_SUFFIX}"
21+
"APC_LITERAL": "${APC_UNKNOWN}|$APC_VALUE|${PLUGIN_ROOT_SUFFIX}",
22+
"USER": "apc-configured-environment-precedence",
23+
"USERNAME": "apc-configured-environment-precedence"
2224
}
2325
},
2426
"relative": {

‎plugins/agent-plugins-conformance-core/src/probe.mjs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,30 @@ if (!['default', 'relative', 'root', 'data', 'command-token-posix', 'command-tok
1414
// This value is independent of the client-provided PLUGIN_ROOT environment.
1515
const root = realpathSync.native(fileURLToPath(new URL('..', import.meta.url)));
1616
const environmentNames = ['PLUGIN_ROOT', 'PLUGIN_DATA', 'APC_VALUE', 'APC_EXPANSION', 'APC_LITERAL'];
17+
const environmentPrecedenceNames = ['USER', 'USERNAME'];
1718
let resolvedData = null;
1819
if (process.env.PLUGIN_DATA && isAbsolute(process.env.PLUGIN_DATA)) {
1920
try { resolvedData = realpathSync.native(process.env.PLUGIN_DATA); } catch { /* Unobservable target remains null. */ }
2021
}
22+
const environment = Object.fromEntries(environmentNames.filter((name) => process.env[name] !== undefined)
23+
.map((name) => [name, process.env[name]]));
24+
if (['default', 'data'].includes(serverName)) {
25+
for (const name of environmentPrecedenceNames) environment[name] = process.env[name] ?? null;
26+
}
2127
const launch = {
2228
server: serverName,
2329
root,
2430
resolvedData,
2531
// Compare directory identity even when the OS retains a junction/alias spelling.
2632
cwd: realpathSync.native(process.cwd()),
2733
argv: process.argv.slice(2),
28-
env: Object.fromEntries(environmentNames.filter((name) => process.env[name] !== undefined)
29-
.map((name) => [name, process.env[name]])),
34+
env: environment,
3035
};
3136
const server = new Server({ name: `agent-plugins-conformance-${serverName}`, version: '0.1.0' },
3237
{ capabilities: { tools: {} } });
3338
const observeTool = {
3439
name: 'observe',
35-
description: 'Return this process launch evidence, including only fixture environment variables.' +
40+
description: 'Return this process launch evidence, including selected environment variables.' +
3641
(serverName === 'default' ? ' Each call also creates, writes, closes, and removes a uniquely named temporary file directly in an absolute PLUGIN_DATA directory, recording any operation or cleanup error.' : '') +
3742
' Record the observation object from structuredContent (or parsed JSON text) unchanged with the run-conformance reporter; exclude the MCP result wrapper.',
3843
inputSchema: { type: 'object', properties: {}, additionalProperties: false },

‎plugins/agent-plugins-conformance/src/cases.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export const CASES = Object.freeze([
2727
['filesystem.data.distinct-across-plugins', 'Distinct data directories across plugins', ['9.1']],
2828
['filesystem.data.consistent-within-plugin', 'Consistent data directory within a plugin', ['9.1']],
2929
['mcp.stdio.env.configured-value', 'Configured environment', ['9.1']],
30+
['mcp.stdio.env.configured-precedence', 'Configured environment precedence', ['9.1']],
3031
['mcp.stdio.command.single-token', 'Command token preservation', ['7.2.1']],
3132
['mcp.stdio.args.preservation-and-expansion', 'Argument preservation and expansion', ['7.2.1', '9.2']],
3233
['mcp.stdio.env.expansion', 'Environment expansion', ['9.2']],

‎plugins/agent-plugins-conformance/src/report.mjs‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,12 @@ const SKILLS = {
5555
'conformance-recovery-valid': 'APC_RECOVERY_VALID_V1',
5656
'conformance-invalid-mcp-valid': 'APC_INVALID_MCP_VALID_V1',
5757
};
58-
const ENV_KEYS = ['PLUGIN_ROOT', 'PLUGIN_DATA', 'APC_VALUE', 'APC_EXPANSION', 'APC_LITERAL'];
58+
const ENVIRONMENT_PRECEDENCE_KEYS = ['USER', 'USERNAME'];
59+
const ENVIRONMENT_PRECEDENCE_VALUE = 'apc-configured-environment-precedence';
60+
const ENV_KEYS = [
61+
'PLUGIN_ROOT', 'PLUGIN_DATA', 'APC_VALUE', 'APC_EXPANSION', 'APC_LITERAL',
62+
...ENVIRONMENT_PRECEDENCE_KEYS,
63+
];
5964

6065
function invalid(at, reason) {
6166
throw new TypeError(`${at}: ${reason}`);
@@ -295,7 +300,10 @@ export function validateInput(input, { recording = false } = {}) {
295300
array(evidence.argv, `${evidenceAt}.argv`);
296301
evidence.argv.forEach((argument, i) => string(argument, `${evidenceAt}.argv[${i}]`, true));
297302
object(evidence.env, ENV_KEYS, [], `${evidenceAt}.env`);
298-
for (const [key, value] of Object.entries(evidence.env)) string(value, `${evidenceAt}.env.${key}`, true);
303+
for (const [key, value] of Object.entries(evidence.env)) {
304+
if (ENVIRONMENT_PRECEDENCE_KEYS.includes(key) && value === null) continue;
305+
string(value, `${evidenceAt}.env.${key}`, true);
306+
}
299307
if (observation.server === 'default' && evidence.dataWrite !== null) {
300308
const at = `${evidenceAt}.dataWrite`;
301309
const write = evidence.dataWrite;
@@ -716,6 +724,37 @@ export function buildReport(input) {
716724
`default resolved PLUGIN_DATA to ${JSON.stringify(defaultData)}. data resolved PLUGIN_DATA to ${JSON.stringify(dataCwdData)}.`);
717725
}
718726
const evidence = runtime.get('default');
727+
const precedenceCase = 'mcp.stdio.env.configured-precedence';
728+
const environmentPrecedenceEvidence = Object.fromEntries(['default', 'data']
729+
.map((server) => [server, runtime.get(server)]));
730+
const missingPrecedenceServers = Object.entries(environmentPrecedenceEvidence)
731+
.filter(([, value]) => value === undefined).map(([server]) => server);
732+
const missingPrecedenceKeys = Object.entries(environmentPrecedenceEvidence).flatMap(([server, value]) => value === undefined
733+
? []
734+
: ENVIRONMENT_PRECEDENCE_KEYS.filter((key) => !Object.hasOwn(value.env, key))
735+
.map((key) => `${server}.${key}`));
736+
if (missingPrecedenceServers.length > 0) {
737+
set(precedenceCase, 'not_verified',
738+
`Missing MCP observation for: ${missingPrecedenceServers.join(', ')}.`);
739+
} else if (missingPrecedenceKeys.length > 0) {
740+
set(precedenceCase, 'not_verified',
741+
`Environment precedence evidence is missing: ${missingPrecedenceKeys.join(', ')}.`);
742+
} else {
743+
const configuredDifferences = ENVIRONMENT_PRECEDENCE_KEYS
744+
.filter((key) => environmentPrecedenceEvidence.default.env[key] !== ENVIRONMENT_PRECEDENCE_VALUE)
745+
.map((key) => mismatch(key, ENVIRONMENT_PRECEDENCE_VALUE, environmentPrecedenceEvidence.default.env[key]));
746+
if (configuredDifferences.length > 0) {
747+
set(precedenceCase, 'fail',
748+
`Configured environment values were not delivered. ${configuredDifferences.join(' ')}`);
749+
} else {
750+
const differingControls = ENVIRONMENT_PRECEDENCE_KEYS.filter((key) =>
751+
typeof environmentPrecedenceEvidence.data.env[key] === 'string' &&
752+
environmentPrecedenceEvidence.data.env[key] !== ENVIRONMENT_PRECEDENCE_VALUE);
753+
set(precedenceCase, differingControls.length > 0 ? 'pass' : 'not_verified', differingControls.length > 0
754+
? `Configured values replaced different inherited control values for: ${differingControls.join(', ')}.`
755+
: 'No control value differed from the configured value, so replacement was not observable.');
756+
}
757+
}
719758
if (evidence) {
720759
const { root, env, argv } = evidence;
721760
const flavor = pathFlavor(root);

‎test/probe.test.mjs‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ async function connect(fixture, mode, override = {}) {
3737
const client = new Client({ name: 'conformance-reference-test', version: '1' });
3838
const transport = new StdioClientTransport({
3939
command: process.execPath, args: config.args.map(expand), cwd,
40-
env: { ...variables, ...Object.fromEntries(Object.entries(config.env ?? {}).map(([key, value]) => [key, expand(value)])), APC_SHOULD_NOT_LEAK: 'unrelated ambient sentinel' },
40+
env: { USER: 'ambient-user', USERNAME: 'ambient-username', ...variables,
41+
...Object.fromEntries(Object.entries(config.env ?? {}).map(([key, value]) => [key, expand(value)])),
42+
APC_SHOULD_NOT_LEAK: 'unrelated ambient sentinel' },
4143
stderr: 'pipe', ...override,
4244
});
4345
fixture.clients.push(client);
@@ -61,6 +63,16 @@ test('copied plugin runs only MCP observation tools without node_modules', { tim
6163
assert.deepEqual(result.structuredContent, observation);
6264
assert.equal(observation.evidence.root, files.root);
6365
assert.equal(observation.evidence.env.APC_SHOULD_NOT_LEAK, undefined);
66+
if (mode === 'default') {
67+
assert.equal(observation.evidence.env.USER, 'apc-configured-environment-precedence');
68+
assert.equal(observation.evidence.env.USERNAME, 'apc-configured-environment-precedence');
69+
} else if (mode === 'data') {
70+
assert.equal(observation.evidence.env.USER, 'ambient-user');
71+
assert.equal(observation.evidence.env.USERNAME, 'ambient-username');
72+
} else {
73+
assert.equal(Object.hasOwn(observation.evidence.env, 'USER'), false);
74+
assert.equal(Object.hasOwn(observation.evidence.env, 'USERNAME'), false);
75+
}
6476
if (mode === 'default') {
6577
assert.equal(dirname(observation.evidence.dataWrite.path), files.data);
6678
assert.equal(observation.evidence.dataWrite.error, null);
@@ -93,10 +105,34 @@ test('copied plugin runs only MCP observation tools without node_modules', { tim
93105
const reportInput = input(observations);
94106
const direct = buildReport(reportInput);
95107
assert.equal(direct.summary.fail, 0);
96-
assert.equal(direct.summary.pass, 15);
108+
assert.equal(direct.summary.pass, 16);
97109
assert.equal(direct.summary.not_verified, 41);
98110
});
99111

112+
test('faulty ambient precedence becomes a normal reporter failure for both platform variables', { timeout: 30_000 }, async (t) => {
113+
const files = await fixture(t);
114+
const variables = { PLUGIN_ROOT: files.root, PLUGIN_DATA: files.data };
115+
const expand = (value) => value.replace(/\$\{(PLUGIN_ROOT|PLUGIN_DATA)\}/g, (_, name) => variables[name]);
116+
const configured = Object.fromEntries(Object.entries(files.config.mcpServers.default.env)
117+
.map(([name, value]) => [name, expand(value)]));
118+
for (const name of ['USER', 'USERNAME']) {
119+
const control = await connect(files, 'data', { env: {
120+
...variables, USER: 'ambient-user', USERNAME: 'ambient-username',
121+
} });
122+
const faulty = await connect(files, 'default', { env: {
123+
...variables, ...configured, [name]: `ambient-${name.toLowerCase()}`,
124+
} });
125+
const [controlResult, faultyResult] = await Promise.all([
126+
control.callTool({ name: 'observe', arguments: {} }),
127+
faulty.callTool({ name: 'observe', arguments: {} }),
128+
]);
129+
const report = buildReport(input([faultyResult.structuredContent, controlResult.structuredContent]));
130+
const outcome = report.results.find(({ id }) => id === 'mcp.stdio.env.configured-precedence');
131+
assert.equal(outcome.status, 'fail', name);
132+
assert.match(outcome.detail, new RegExp(`${name}: expected`));
133+
}
134+
});
135+
100136
test('copied recovery plugin serves exact valid and invalid-server observations without runtime dependencies', { timeout: 30_000 }, async (t) => {
101137
const parent = await mkdtemp(join(tmpdir(), 'agent-plugins-conformance-recovery-'));
102138
const root = join(parent, 'copied recovery plugin');

‎test/record-report.test.mjs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,16 @@ const mcp = () => ({
2020
env: {
2121
PLUGIN_ROOT: '/plugin', PLUGIN_DATA: '/data', APC_VALUE: 'fixture value with spaces',
2222
APC_EXPANSION: '/plugin|/data|/plugin', APC_LITERAL: '${APC_UNKNOWN}|$APC_VALUE|${PLUGIN_ROOT_SUFFIX}',
23+
USER: 'apc-configured-environment-precedence', USERNAME: 'apc-configured-environment-precedence',
2324
},
2425
},
2526
});
27+
const dataMcp = (env = { USER: null, USERNAME: 'ambient-username' }) => ({
28+
kind: 'mcp-stdio', server: 'data', evidence: {
29+
version: 1, server: 'data', root: '/plugin', cwd: '/data', resolvedData: '/data',
30+
argv: ['data'], env,
31+
},
32+
});
2633
const commandToken = (platform = 'posix', {
2734
root = platform === 'posix' ? '/plugin' : 'C:\\plugin',
2835
data = platform === 'posix' ? '/data' : 'D:\\data',
@@ -393,6 +400,23 @@ test('distinct keys accumulate in deterministic order; repeated keys fully repla
393400
assert.deepEqual(await readdir(join(f.directory, 'nested directory')), ['report with spaces.json']);
394401
});
395402

403+
test('recorder preserves nullable environment precedence evidence through canonical reloads', async (t) => {
404+
const f = await fixture(t);
405+
f.success(start);
406+
f.record(dataMcp());
407+
f.record(mcp());
408+
let report = await f.read();
409+
assert.deepEqual(report.observations.filter(({ server }) => ['default', 'data'].includes(server)), [mcp(), dataMcp()]);
410+
assert.equal(report.results.find(({ id }) => id === 'mcp.stdio.env.configured-precedence').status, 'pass');
411+
assert.equal(report.observations.find(({ server }) => server === 'data').evidence.env.USER, null);
412+
413+
const oldData = dataMcp({});
414+
f.record(oldData);
415+
report = await f.read();
416+
assert.deepEqual(report.observations.find(({ server }) => server === 'data'), oldData);
417+
assert.equal(report.results.find(({ id }) => id === 'mcp.stdio.env.configured-precedence').status, 'not_verified');
418+
});
419+
396420
test('command-token evidence records canonically and replacement changes only its verdict', async (t) => {
397421
const f = await fixture(t);
398422
f.success(start);

‎test/report-human.test.mjs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { formatReport } from '../plugins/agent-plugins-conformance/src/report-fo
66
function input() {
77
const root = '/fixture/plugin';
88
const data = '/state/plugin';
9+
const precedenceEnv = (server) => server === 'default'
10+
? { USER: 'apc-configured-environment-precedence', USERNAME: 'apc-configured-environment-precedence' }
11+
: server === 'data' ? { USER: 'ambient-user', USERNAME: 'ambient-username' } : {};
912
return {
1013
schemaVersion: 1,
1114
observations: [
@@ -18,7 +21,8 @@ function input() {
1821
cwd: server === 'default' ? root : server === 'data' ? data : `${root}/probe-workdir`,
1922
argv: ['default', 'arg with spaces', '', root, data, '${APC_UNKNOWN}', '$APC_VALUE', '${PLUGIN_ROOT_SUFFIX}'],
2023
env: { PLUGIN_ROOT: root, PLUGIN_DATA: data, APC_VALUE: 'fixture value with spaces',
21-
APC_EXPANSION: `${root}|${data}|${root}`, APC_LITERAL: '${APC_UNKNOWN}|$APC_VALUE|${PLUGIN_ROOT_SUFFIX}' },
24+
APC_EXPANSION: `${root}|${data}|${root}`, APC_LITERAL: '${APC_UNKNOWN}|$APC_VALUE|${PLUGIN_ROOT_SUFFIX}',
25+
...precedenceEnv(server) },
2226
},
2327
})),
2428
{
@@ -59,9 +63,9 @@ test('complete stdio and recovery evidence leaves optional HTTP and SSE checks u
5963
'',
6064
'Checks Passed Failed Not verified',
6165
'Skills 3 0 0',
62-
'MCP 22 0 26',
66+
'MCP 23 0 26',
6367
'Filesystem 4 0 1',
64-
'Total 29 0 27',
68+
'Total 30 0 27',
6569
].join('\n'));
6670
const missing = human.split('\n\nNot verified\n')[1];
6771
for (const id of [
@@ -100,7 +104,7 @@ test('all-unverified human report preserves every saved missing-evidence result'
100104
value.observations = [];
101105
const human = formatReport(buildReport(value));
102106
assert.match(human, /Skills\s+0\s+0\s+3/);
103-
assert.match(human, /MCP\s+0\s+0\s+48/);
107+
assert.match(human, /MCP\s+0\s+0\s+49/);
104108
assert.match(human, /Filesystem\s+0\s+0\s+5/);
105109
assert.match(human, /Missing skill observations: conformance-alpha, conformance-beta\./);
106110
for (const result of buildReport(value).results) assert.ok(human.includes(`(${result.id})`));
@@ -112,7 +116,7 @@ test('mixed human report puts failures before missing-evidence details', () => {
112116
value.observations = [value.observations[2]];
113117
value.observations[0].evidence.env.APC_VALUE = 'incorrect configured value';
114118
const human = formatReport(buildReport(value));
115-
assert.match(human, /MCP\s+6\s+1\s+41/);
119+
assert.match(human, /MCP\s+6\s+1\s+42/);
116120
assert.match(human, /Filesystem\s+1\s+0\s+4/);
117121
assert.match(human, /Configured environment \(mcp.stdio.env.configured-value\)/);
118122
assert.match(human, /"fixture value with spaces"/);
@@ -169,7 +173,7 @@ test('human report renders saved warnings independently of result status', () =>
169173
const writable = value.results.find(({ id }) => id === 'filesystem.data.writable');
170174
writable.status = 'not_verified';
171175
writable.warning = 'Saved cleanup warning for /state/plugin/leftover.';
172-
value.summary = { pass: 16, fail: 0, not_verified: 40, total: 56 };
176+
value.summary = { pass: 17, fail: 0, not_verified: 40, total: 57 };
173177
value.observations = [];
174178

175179
const human = formatReport(value);

0 commit comments

Comments
 (0)