fix(deploy): ACTION cannot seed by default, and the probe stops overwriting

Five defects from review, all mine.

ACTION is no longer defaulted.  `%let action=MAKEDATA;` meant a caller with no
input table at all ran the seeding body - so a URL that never mentioned ACTION
would create the control library.  The default is MISSING, which is refused,
and a URL parameter is whitelisted through the same check with symget() so it
is never resolved as macro text.

The write probe no longer has a predictable name.  fopen with 'O' truncates, so
a probe called .dcprobe_<pid> destroyed any pre-existing file of that name in
the folder being tested.  It now carries the pid and a random suffix, checks
fileexist before use, and gives up rather than reuse a name.

DIRLIST returns folders and locked entries only.  The old filter dropped 'file'
and passed every other value - blanks and unknown types - to the picker as a
folder.  The filter is now an explicit whitelist, and the header describes what
is actually emitted.

A request with no input table is refused, covered by a test: that is the case
the default used to swallow.

ngOnDestroy is gone from the client component - it reset a flag on a component
being torn down.

The deploy-window exposure is recorded in .agents/docs/, with why the admin
group cannot be checked there: %mpeinit() reads &mpeadmins from the control
library that makedata creates, so before it runs there is no group to check -
which is also why the previous dirlist could never serve this screen.
This commit is contained in:
dc committed 2026-11-03 11:59:24 +00:00
1 parent 4424171b2f
commit e84cfaaabd
6 files changed
+105 -39

No files matched your search

+35
View File
@@ -0,0 +1,35 @@
# The deploy window: who can browse the filesystem before Data Controller is configured
The deploy screens ask two questions about the physical filesystem - what folders sit below this parent, and can the deployment user write to this one. Both are answered by `makedata` (`ACTION=DIRLIST` and `ACTION=WRITECHECK`), through `mpe_adminsetup.sas`.
This note records the exposure that comes with that, so it is a decision rather than an oversight.
## The window
`makedata` exists from the moment the service pack is deployed until it runs successfully, at which point it deletes itself (and, on SASjs Server, the configurator). Those two actions are therefore reachable in that window by any caller who can reach the STPs at all.
What that caller can do:
- `DIRLIST` - enumerate the folders below any absolute path the SAS process can open. Folder names only; no file contents, no file listing.
- `WRITECHECK` - ask whether a path exists, is a folder, and whether the deployment user can create and delete a file in it. It creates a uniquely named probe file and removes it again.
Both refuse a value carrying a macro trigger, a quote or a statement separator, and shape check it with `mp_validatecol(ISPATH)` before use.
## Why there is no group check
Every other admin service gates on `&mpeadmins`, which comes from `MPE_CONFIG` in the control library - and that library is what `makedata` creates. Before it runs there is no group to check: `%mpeinit()` reads `&DC_LIBREF..mpe_config` and has nothing to read. That is also why the previous `dirlist` service could never serve the deploy screen it was written for.
The window is not narrower than the configurator's own: on SASjs Server the configurator page, in the same window, lets the caller pick any group they belong to and create the control library - which is full Data Controller admin. `DIRLIST` and `WRITECHECK` are strictly less powerful than that.
## What bounds it
`makedata` self-destructs after a successful run, so a configured deployment has no service that can enumerate a directory or write a probe file. The exposure exists only between deploying the pack and running `makedata`.
Two things follow for an operator:
- run `makedata` as part of the same session as the deploy rather than leaving a deployed but unconfigured pack in place;
- if a deployment is abandoned mid-flight, remove the service pack, which removes `makedata` with it.
## If it needs to be tighter
The remaining option is a deployment-time gate that does not depend on the control library - for example an allowlist of callers supplied in the service (or an OS/metadata group named before deployment). That is a change to how the deploy screens authenticate, not a change to this service, and it is deliberately not part of it.
@@ -1,10 +1,4 @@
import {
Component,
EventEmitter,
Input,
OnDestroy,
Output
} from '@angular/core'
import { Component, EventEmitter, Input, Output } from '@angular/core'
import {
DcFolderEntry,
DcFolderRow,
@@ -33,7 +27,7 @@ import { SasService } from 'src/app/services/sas.service'
styleUrls: ['./dc-location.component.scss'],
standalone: false
})
export class DcLocationComponent implements OnDestroy {
export class DcLocationComponent {
@Input() path = ''
@Input() inputId = 'dcloc'
@Output() pathChange = new EventEmitter<string>()
@@ -55,10 +49,6 @@ export class DcLocationComponent implements OnDestroy {
constructor(private sasService: SasService) {}
ngOnDestroy() {
this.browsing = false
}
/**
* SAS has no zero length character value, so a blank field arrives as a single
* space. Everything read from the service goes through here before it is
+28 -13
View File
@@ -14,8 +14,9 @@
mp_dirlist's job, so no filesystem logic lives here. DEPTH (1-3, default 1)
is how many levels come back, so a parent with tens of thousands of
subfolders is never transferred in full, and MAXOBS (default 2000) caps the
rows with a TRUNCATED flag. Only folders are offered; one that cannot be
opened is kept and marked `locked` so the administrator can see it is there.
rows with a TRUNCATED flag. Only rows mp_dirlist classified as a folder or
as `locked` come back - a `locked` one is an entry that could not be opened,
kept so the administrator can see it is there, and named in FILE_OR_FOLDER.
A finite depth is also what makes a symlink loop harmless.
`WRITECHECK` answers the other half, behind its own button: does the path
@@ -150,7 +151,7 @@
into the platform roots by mp_dirlist.
*/
data work.dirlist;
set work.dirraw(where=(file_or_folder ne 'file'));
set work.dirraw(where=(upcase(file_or_folder) in ('FOLDER','LOCKED')));
retain _kept 0;
_kept=_kept+1;
if _kept>&br_maxobs then do;
@@ -261,17 +262,31 @@
if is_folder='Y' then do;
if index(path,'\')>0 then sep='\';
else sep='/';
probe=cats(path,sep,'.dcprobe_',symget('sysprocessid'));
rc=filename('dc_probe',probe);
fid=fopen('dc_probe','O');
if fid>0 then do;
rc=fclose(fid);
rc=fdelete('dc_probe');
writeable='Y';
msg='';
/*
The probe name carries the process id and a random number, and an
existing file is never reused: fopen with 'O' truncates, so a fixed
name would destroy whatever was already called that.
*/
probe='';
do _try=1 to 10;
probe=cats(path,sep,'.dcprobe_',symget('sysprocessid'),'_'
,put(int(ranuni(0)*1e9),z10.));
if fileexist(probe)=0 then leave;
probe='';
end;
if missing(probe) then msg='No free name for the write probe';
else do;
rc=filename('dc_probe',probe);
fid=fopen('dc_probe','O');
if fid>0 then do;
rc=fclose(fid);
rc=fdelete('dc_probe');
writeable='Y';
msg='';
end;
else msg=sysmsg();
rc=filename('dc_probe');
end;
else msg=sysmsg();
rc=filename('dc_probe');
end;
run;
@@ -61,21 +61,26 @@
a macro variable - the column comes from the request, so anything else is
rejected rather than resolved as macro text.
*/
%let action=MAKEDATA;
%let action=MISSING;
%macro check_action();
%if &exist=1 %then %do;
/*
ACTION is required. It arrives either as a column of the input table or, for
the configurator's URL, as a macro variable - and both are whitelisted here,
because the URL value is caller controlled and must never be resolved as
macro text. symget() reads it without resolving it.
*/
data _null_;
set work.fromjs(obs=1);
length _act $10;
/* ACTION is required. It is not defaulted: a caller that does not say what
it wants gets a clear refusal rather than a seeded control library. */
_act='MISSING';
if not missing(action) then _act=upcase(strip(cats(action)));
if &exist=1 then do;
set work.fromjs(obs=1);
if not missing(action) then _act=upcase(strip(cats(action)));
end;
else _act=upcase(strip(symget('action')));
if _act='MISSING' or _act in ('MAKEDATA','DIRLIST','WRITECHECK')
then call symputx('action',_act,'g');
else call symputx('action','BAD','g');
run;
%end;
%mend check_action;
%check_action()
@@ -387,6 +387,22 @@ run;
outds=work.test_results
)
/**
* Test 16 - no input table at all: ACTION is not defaulted, so a caller that
sends nothing cannot fall through to the seeding body
*/
%mpe_execute(&_program,
viyacontext=&defaultcontext,
outlib=web11,
failfast=0
)
%mpe_assert_abort(
desc=A request with no input table cannot seed,
outstatus=work.dc_childstatus,
msg=%str(ACTION is required: MAKEDATA, DIRLIST or WRITECHECK),
outds=work.test_results
)
/**
* dump results to the log for offline inspection
*/
@@ -56,21 +56,26 @@
a macro variable - the column comes from the request, so anything else is
rejected rather than resolved as macro text.
*/
%let action=MAKEDATA;
%let action=MISSING;
%macro check_action();
%if &exist=1 %then %do;
/*
ACTION is required. It arrives either as a column of the input table or, for
the configurator's URL, as a macro variable - and both are whitelisted here,
because the URL value is caller controlled and must never be resolved as
macro text. symget() reads it without resolving it.
*/
data _null_;
set work.fromjs(obs=1);
length _act $10;
/* ACTION is required. It is not defaulted: a caller that does not say what
it wants gets a clear refusal rather than a seeded control library. */
_act='MISSING';
if not missing(action) then _act=upcase(strip(cats(action)));
if &exist=1 then do;
set work.fromjs(obs=1);
if not missing(action) then _act=upcase(strip(cats(action)));
end;
else _act=upcase(strip(symget('action')));
if _act='MISSING' or _act in ('MAKEDATA','DIRLIST','WRITECHECK')
then call symputx('action',_act,'g');
else call symputx('action','BAD','g');
run;
%end;
%mend check_action;
%check_action()