fix(deploy): gate the browse actions on the settings file

.agents/docs/deploy-window-authorization.md

@@ -1,19 +1,14 @@
-# Why the deploy window has no admin gate
+# What guards the deploy window
 
-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`.
+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_dirlist.sas` and `mpe_writecheck.sas`.
 
-Neither action checks a group, and this note records why that is the right answer rather than an oversight - so the next review does not have to work it out again.
+This note records the two-layer answer: a gate that closes the window once the appLoc is configured, and why the window itself has no group check.
 
-## The window is the deployment itself
+## The gate
 
-`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). Deploying the pack and running `makedata` is an administrator's job, done in one sitting; there is no steady state in which these actions are reachable on a configured system.
+The deploy window is over once the appLoc has `services/public/settings.sas`: `makedata` writes it at the end of a successful run, and a run that failed - or was abandoned - after writing it leaves the file behind. So the service refuses `DIRLIST` and `WRITECHECK` whenever that file is present, however the run that created it ended. On SASjs Server the file is fetched with `ms_getfile` and a probe that cannot answer (anything other than 200 or 404) is treated as configured: an unreachable drive is not evidence that the window is open. Viya checks the same file through `mfv_existfile`.
 
-What the actions do, for the record:
-
-- `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.
+The consequence for tests: the browse actions are refused on any appLoc that runs the suite, because every such appLoc is configured. `services/admin/makedata.test.sas` therefore asserts the refusal, and the listing and write-probe behaviour is asserted against the macros directly (`macros/mpe_dirlist.test.sas`, `macros/mpe_writecheck.test.sas`). The macros' own abort paths (a relative path, one carrying a macro trigger) are not asserted anywhere: `mp_abort` cannot be exercised inside the test session.
 
 ## Why there is no group check
 
@@ -21,13 +16,17 @@ Every other admin service gates on `&mpeadmins`, which comes from `MPE_CONFIG` i
 
 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 the actions do, for the record
+
+- `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. The shape check fails closed: an absent `ISPATH` rule refuses the path rather than accepting it.
+
 ## The assessment
 
-- **There is no admin group to check.** It is created by `makedata`, from the group the deployer picks in the configurator.
+- **The window is the deployment itself.** `makedata` is deployed, run once by an administrator, and deletes itself on success.
+- **There is no admin group to check while it runs.** It is created by `makedata`, from the group the deployer picks in the configurator.
 - **The caller is the administrator.** Reaching these actions means reaching the STPs of a deployment in progress, which is the same caller the configurator is serving in the same window.
 - **The actions are the weaker half of that window.** The configurator, in the same window, lets the caller pick any group they belong to and create the control library - which is full Data Controller admin. Listing folders and probing a write cannot achieve that.
-- **They do not survive.** A successful `makedata` deletes the service, so a configured system has nothing that can enumerate a directory or write a probe file.
-
-## If it ever stops being internal
-
-The option, if these actions are ever reachable outside a deployment, is a gate that does not depend on the control library - an allowlist in the service, or an OS/metadata group named at deploy time. That is a change to how the deploy screens authenticate rather than a change to this service.
+- **The window closes on the settings file, not on survival of the service.** That is what covers a failed or abandoned run.

client/src/app/deploy/sections/dc-location/dc-location.component.html

@@ -4,13 +4,14 @@
       clrInput
       [id]="inputId"
       [name]="inputId"
+      [attr.aria-label]="ariaLabel"
       [ngModel]="path"
       (ngModelChange)="onPathChange($event)"
       placeholder="Type a folder, or browse below"
     />
     <button
       type="button"
-      id="DCBrowse"
+      id="{{ inputId }}-browse"
       class="btn btn-sm btn-outline dc-location-button"
       (click)="toggleBrowse()"
     >
@@ -18,7 +19,7 @@
     </button>
     <button
       type="button"
-      id="DCValidate"
+      id="{{ inputId }}-validate"
       class="btn btn-sm btn-outline dc-location-button"
       [disabled]="validating"
       (click)="validate()"
@@ -34,7 +35,7 @@
 
   @if (checkResult) {
     <p
-      id="DCValidateResult"
+      id="{{ inputId }}-validate-result"
       class="dc-location-result"
       [class.dc-location-ok]="checkResult.writeable"
       [class.dc-location-bad]="!checkResult.writeable"
@@ -51,11 +52,11 @@
   }
 
   @if (browsing) {
-    <div class="dc-browser" id="DCBrowser">
+    <div class="dc-browser" id="{{ inputId }}-browser">
       <div class="dc-browser-bar">
         <button
           type="button"
-          id="DCUp"
+          id="{{ inputId }}-up"
           class="btn btn-sm btn-link dc-browser-up"
           [disabled]="listingIsRoot"
           (click)="up()"
@@ -73,7 +74,7 @@
         @if (!listingIsRoot) {
           <button
             type="button"
-            id="DCUseCurrent"
+            id="{{ inputId }}-use-current"
             class="btn btn-sm btn-outline dc-browser-use"
             (click)="useCurrent()"
           >
@@ -89,7 +90,7 @@
       }
 
       @if (!loading && !!listingError) {
-        <p class="dc-location-bad" id="DCBrowserError">
+        <p class="dc-location-bad" id="{{ inputId }}-browser-error">
           <clr-icon shape="exclamation-triangle"></clr-icon>
           <span>{{ listingError }}</span>
         </p>
@@ -130,7 +131,7 @@
             </li>
           }
           @if (!folders.length) {
-            <li class="dc-folder dc-folder-empty" id="DCNoFolders">
+            <li class="dc-folder dc-folder-empty" id="{{ inputId }}-no-folders">
               No folders here
             </li>
           }
@@ -138,7 +139,7 @@
       }
 
       @if (truncated) {
-        <p class="dc-browser-truncated" id="DCTruncated">
+        <p class="dc-browser-truncated" id="{{ inputId }}-truncated">
           Only the first folders are shown - type the path if it is not listed.
         </p>
       }

client/src/app/deploy/sections/dc-location/dc-location.component.spec.ts

@@ -1,4 +1,4 @@
-import { NO_ERRORS_SCHEMA } from '@angular/core'
+import { Component, NO_ERRORS_SCHEMA } from '@angular/core'
 import { ComponentFixture, TestBed } from '@angular/core/testing'
 import { FormsModule } from '@angular/forms'
 import { SasService } from 'src/app/services/sas.service'
@@ -11,15 +11,46 @@ describe('DcLocationComponent', () => {
   let requests: { url: string; data: any }[]
   let response: any
   let rejection: any
+  let defer: boolean
+  let pending: {
+    url: string
+    data: any
+    resolve: (value: any) => void
+    reject: (reason: any) => void
+  }[]
 
   const sasService = {
     request: (url: string, data: any) => {
       requests.push({ url, data })
+      if (defer) {
+        return new Promise((resolve, reject) => {
+          pending.push({ url, data, resolve, reject })
+        })
+      }
       if (rejection) return Promise.reject(rejection)
       return Promise.resolve(response)
     }
   }
 
+  /**
+   * Hosts the picker so the spec can set `inputId` the way a deploy screen
+   * does - the element ids are only observable through the parent binding.
+   */
+  @Component({
+    selector: 'app-dc-location-host',
+    template: `<app-dc-location
+      [(path)]="path"
+      [inputId]="inputId"
+      [ariaLabel]="ariaLabel"
+    ></app-dc-location>`,
+    standalone: false
+  })
+  class DcLocationHostComponent {
+    path = ''
+    inputId = 'dcloc'
+    ariaLabel = 'Data Controller location'
+  }
+
   const dirResponse = (parent: string, rows: any[], extra: any = {}) => ({
     adapterResponse: {
       dirinfo: [
@@ -48,9 +79,11 @@ describe('DcLocationComponent', () => {
     requests = []
     response = undefined
     rejection = undefined
+    defer = false
+    pending = []
 
     TestBed.configureTestingModule({
-      declarations: [DcLocationComponent],
+      declarations: [DcLocationComponent, DcLocationHostComponent],
       imports: [FormsModule],
       providers: [{ provide: SasService, useValue: sasService }],
       schemas: [NO_ERRORS_SCHEMA]
@@ -308,12 +341,12 @@ describe('DcLocationComponent', () => {
     component.path = '/tmp'
 
     fixture.detectChanges()
-    expect(fixture.nativeElement.querySelector('#DCBrowser')).toBeNull()
+    expect(fixture.nativeElement.querySelector('#dcloc-browser')).toBeNull()
 
     component.toggleBrowse()
     await fixture.whenStable()
     fixture.detectChanges()
-    expect(fixture.nativeElement.querySelector('#DCBrowser')).toBeTruthy()
+    expect(fixture.nativeElement.querySelector('#dcloc-browser')).toBeTruthy()
 
     response = {
       adapterResponse: {
@@ -332,8 +365,93 @@ describe('DcLocationComponent', () => {
     await fixture.whenStable()
     fixture.detectChanges()
 
-    const verdict = fixture.nativeElement.querySelector('#DCValidateResult')
+    const verdict = fixture.nativeElement.querySelector(
+      '#dcloc-validate-result'
+    )
     expect(verdict).toBeTruthy()
     expect(verdict.textContent).toContain('is writeable')
   })
+
+  it('labels the input for assistive tech, from the input', () => {
+    const hostFixture = TestBed.createComponent(DcLocationHostComponent)
+    hostFixture.detectChanges()
+
+    const input = (): HTMLInputElement =>
+      hostFixture.nativeElement.querySelector('#dcloc')
+
+    expect(input()).toBeTruthy()
+    expect(input().getAttribute('aria-label')).toBe('Data Controller location')
+
+    hostFixture.componentInstance.ariaLabel = 'Choose a staging folder'
+    hostFixture.detectChanges()
+
+    expect(input().getAttribute('aria-label')).toBe('Choose a staging folder')
+  })
+
+  it('ignores a listing that arrives after a newer one', async () => {
+    defer = true
+    component.browsing = true
+
+    component.list('a')
+    component.list('b')
+    expect(pending.length).toBe(2)
+
+    /* the newer request answers first, then the older one lands late */
+    pending[1].resolve(dirResponse('/b', [folder('/b/bb', 1, 'bb')]))
+    await fixture.whenStable()
+
+    pending[0].resolve(dirResponse('/a', [folder('/a/aa', 1, 'aa')]))
+    await fixture.whenStable()
+    fixture.detectChanges()
+
+    expect(component.listingParent).toBe('/b')
+    expect(component.folders.map((entry) => entry.filename)).toEqual(['bb'])
+
+    const rendered = Array.from(
+      fixture.nativeElement.querySelectorAll('.dc-folder')
+    ).map((row: any) => row.textContent)
+    expect(rendered.join(' ')).toContain('bb')
+    expect(rendered.join(' ')).not.toContain('aa')
+  })
+
+  it('ignores an error from a listing that has been superseded', async () => {
+    defer = true
+    component.browsing = true
+
+    component.list('a')
+    component.list('b')
+
+    pending[1].resolve(dirResponse('/b', [folder('/b/bb', 1, 'bb')]))
+    await fixture.whenStable()
+
+    pending[0].reject({ adapterResponse: { error: 'Invalid path' } })
+    await fixture.whenStable()
+    fixture.detectChanges()
+
+    expect(component.listingError).toBe('')
+    expect(component.loading).toBeFalse()
+    expect(component.listingParent).toBe('/b')
+  })
+
+  it('derives every element id from the inputId', async () => {
+    const hostFixture = TestBed.createComponent(DcLocationHostComponent)
+    hostFixture.componentInstance.inputId = 'otherPicker'
+    hostFixture.detectChanges()
+
+    const host: HTMLElement = hostFixture.nativeElement
+    expect(host.querySelector('#otherPicker-browse')).toBeTruthy()
+    expect(host.querySelector('#otherPicker-validate')).toBeTruthy()
+    expect(host.querySelector('#DCBrowse')).toBeNull()
+    expect(host.querySelector('#DCValidate')).toBeNull()
+
+    host.querySelector<HTMLButtonElement>('#otherPicker-browse')!.click()
+    await hostFixture.whenStable()
+    hostFixture.detectChanges()
+
+    expect(host.querySelector('#otherPicker-browser')).toBeTruthy()
+    expect(host.querySelector('#otherPicker-up')).toBeTruthy()
+    expect(host.querySelector('#otherPicker-no-folders')).toBeTruthy()
+    expect(host.querySelector('#DCBrowser')).toBeNull()
+    expect(host.querySelector('#DCNoFolders')).toBeNull()
+  })
 })

client/src/app/deploy/sections/dc-location/dc-location.component.ts

@@ -30,6 +30,7 @@ import { SasService } from 'src/app/services/sas.service'
 export class DcLocationComponent {
   @Input() path = ''
   @Input() inputId = 'dcloc'
+  @Input() ariaLabel = 'Data Controller location'
   @Output() pathChange = new EventEmitter<string>()
 
   /** panel state */
@@ -47,6 +48,13 @@ export class DcLocationComponent {
 
   private separator = '/'
 
+  /**
+   * Counts the DIRLIST calls made, so a response to an older request can be
+   * recognised and dropped.  Two overlapping requests may arrive in either
+   * order, and the older one must not overwrite the newer listing.
+   */
+  private listRequestId = 0
+
   constructor(private sasService: SasService) {}
 
   /**
@@ -72,6 +80,7 @@ export class DcLocationComponent {
 
   /** Lists one level below `parent`.  A blank parent is the platform roots. */
   public list(parent: string) {
+    const requestId = ++this.listRequestId
     this.loading = true
     this.listingError = ''
 
@@ -85,6 +94,8 @@ export class DcLocationComponent {
         suppressSuccessAbortModal: true
       })
       .then((res: any) => {
+        if (requestId !== this.listRequestId) return
+
         const response = res?.adapterResponse || {}
         const info = (response.dirinfo || [])[0] || {}
         const listedParent = this.trimSas(info.PARENT)
@@ -97,6 +108,8 @@ export class DcLocationComponent {
         this.loading = false
       })
       .catch((err: any) => {
+        if (requestId !== this.listRequestId) return
+
         this.folders = []
         this.loading = false
         this.listingError = this.errorText(err)

client/src/app/deploy/sections/sasjs-configurator/sasjs-configurator.component.html

@@ -18,7 +18,9 @@
   <strong>{{ SYSUSERID }}</strong> can write, on behalf of Data Controller.
 </p>
 
-<label class="mt-20 clr-control-label">DC Staging Directory</label>
+<label for="dcDirectory" class="mt-20 clr-control-label"
+  >DC Staging Directory</label
+>
 <div class="mb-10 clr-control-container">
   <app-dc-location
     [(path)]="dcDirectory"

sas/sasjs/macros/mpe_action.sas

@@ -0,0 +1,71 @@
+/**
+  @file
+  @brief Resolves the ACTION a makedata request is asking for
+  @details
+  makedata answers the deploy screen's filesystem questions as well as creating
+  the configuration tables, so the action has to be resolved before anything
+  runs.  It arrives one of two ways:
+
+  @li as a column of the input table (the adapter route, and the only one that
+    works on SASjs Server - a form field or query parameter is not delivered to
+    the program there);
+  @li as the macro variable `action`, which is what a URL parameter becomes on
+    a platform that does deliver one, and which the configurator's link sets.
+
+  The macro variable is therefore read BEFORE anything is assigned over it: a
+  default written first would overwrite the incoming value and the parameter
+  would never take effect.  A value that is absent is reported as MISSING and
+  one outside the closed list as BAD, so the caller can refuse them with its
+  own messages rather than this macro aborting.
+
+  @param [out] outvar= (action) Global macro variable to receive the action.
+    Set to MAKEDATA, DIRLIST, WRITECHECK, MISSING or BAD.
+
+  @version 9.2
+  @author 4GL Apps Ltd
+  @copyright 4GL Apps Ltd.  This code may only be used within Data Controller
+    and may not be re-distributed or re-sold without the express permission of
+    4GL Apps Ltd.
+
+  @sasjslint {"maxLineLength": 80}
+
+**/
+
+%macro mpe_action(outvar=action);
+%local incoming resolved;
+%let incoming=;
+%let resolved=MISSING;
+
+/* capture any incoming value before this macro assigns its own - symget reads
+  it without resolving macro text in it, and symexist keeps a missing name out
+  of the log */
+data _null_;
+  if symexist('action') then call symputx('incoming',symget('action'),'l');
+run;
+
+/* a column of the input table wins over the macro variable.  The two cases are
+  separate steps: a SET inside an IF is still executed, so the "no input table"
+  case has to be compiled without one */
+%if %sysfunc(exist(work.fromjs))=1 %then %do;
+data _null_;
+  set work.fromjs(obs=1);
+  if not missing(action) then
+    call symputx('incoming',upcase(strip(cats(action))),'l');
+run;
+%end;
+
+data _null_;
+  length _act $10;
+  _act=upcase(strip(symget('incoming')));
+  /* absent is a different refusal from unrecognised */
+  if missing(_act) then _act='MISSING';
+  if _act not in ('MAKEDATA','DIRLIST','WRITECHECK') and _act ne 'MISSING'
+    then _act='BAD';
+  call symputx('resolved',_act,'l');
+run;
+
+data _null_;
+  call symputx("&outvar",symget('resolved'),'g');
+run;
+
+%mend mpe_action;

sas/sasjs/macros/mpe_action.test.sas

@@ -0,0 +1,117 @@
+/**
+  @file
+  @brief Testing mpe_action macro
+
+  <h4> SAS Macros </h4>
+  @li mp_assert.sas
+  @li mpe_action.sas
+
+  @author 4GL Apps Ltd
+  @copyright 4GL Apps Ltd.  This code may only be used within Data Controller
+    and may not be re-distributed or re-sold without the express permission of
+    4GL Apps Ltd.
+
+  @sasjslint {"maxLineLength": 80}
+
+**/
+
+/**
+  * Test 1 - an ACTION column of the input table is used
+  */
+data work.fromjs;
+  length action $10;
+  action='DIRLIST';
+run;
+%let action=;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="DIRLIST"),
+  desc=The action column is used,
+  outds=work.test_results
+)
+
+/**
+  * Test 2 - the value is upcased, so a lower case column value still matches
+  */
+data work.fromjs;
+  length action $10;
+  action='writecheck';
+run;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="WRITECHECK"),
+  desc=The action column is upcased,
+  outds=work.test_results
+)
+
+/**
+  * Test 3 - the column wins over an incoming macro variable, which is what a
+  * URL parameter becomes
+  */
+data work.fromjs;
+  length action $10;
+  action='MAKEDATA';
+run;
+%let action=DIRLIST;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="MAKEDATA"),
+  desc=The action column wins over an incoming value,
+  outds=work.test_results
+)
+
+/**
+  * Test 4 - with no input table the incoming macro variable is read.  This is
+  * the regression the capture exists for: a default assigned before the read
+  * would overwrite it, and this assertion is what catches that.
+  */
+proc datasets lib=work nolist nowarn;
+  delete fromjs;
+quit;
+%let action=WRITECHECK;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="WRITECHECK"),
+  desc=An incoming value is read when there is no input table,
+  outds=work.test_results
+)
+
+/**
+  * Test 5 - nothing supplied at all is MISSING, not a default
+  */
+%let action=;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="MISSING"),
+  desc=An absent action is reported as missing,
+  outds=work.test_results
+)
+
+/**
+  * Test 6 - a value outside the closed list is BAD, so the caller refuses it
+  * rather than resolving it as anything
+  */
+data work.fromjs;
+  length action $10;
+  action='NONSENSE';
+run;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="BAD"),
+  desc=An unrecognised action is reported as bad,
+  outds=work.test_results
+)
+
+/**
+  * Test 7 - and the same through the macro variable route
+  */
+proc datasets lib=work nolist nowarn;
+  delete fromjs;
+quit;
+%let action=NONSENSE;
+%mpe_action()
+%mp_assert(
+  iftrue=("&action"="BAD"),
+  desc=An unrecognised incoming value is reported as bad,
+  outds=work.test_results
+)

sas/sasjs/macros/mpe_dirlist.sas

@@ -91,11 +91,11 @@ data _null_;
   if index(_parent,'(')>0 then _bad=1;
   if index(_parent,')')>0 then _bad=1;
 
-  /* blank is the root, so only a supplied parent is shape checked.
-    Initialised to 1 so a core release without the ISPATH rule leaves it
-    alone */
+  /* blank is the root, so only a supplied parent is shape checked.  The flag
+    starts at 0 so a core release without the ISPATH rule refuses the path
+    rather than accepting it - the value comes from the request */
   if not missing(_parent) then do;
-    is_path=1;
+    is_path=0;
     %mp_validatecol(_parent,ISPATH,is_path)
     if is_path=0 then _bad=1;
   end;

sas/sasjs/macros/mpe_dirlist.test.sas

@@ -2,9 +2,10 @@
   @file
   @brief Testing mpe_dirlist macro
   @details Covers the guard, the listing shape and the cap.  The abort paths
-  (a relative parent, one carrying macro triggers) are covered by
-  services/admin/makedata.test.sas, because mp_abort cannot be exercised
-  inside the test session itself.
+  (a relative parent, one carrying macro triggers) are not asserted anywhere:
+  mp_abort cannot be exercised inside the test session, and the service that
+  calls this macro refuses the browse actions on any appLoc that carries a
+  settings.sas, which every appLoc running the suite does.
 
   <h4> SAS Macros </h4>
   @li mf_getuniquefileref.sas

sas/sasjs/macros/mpe_writecheck.sas

@@ -78,7 +78,8 @@ data _null_;
   if index(_path,')')>0 then _bad=1;
 
   if _bad=0 then do;
-    is_path=1;
+    /* fail closed: an absent ISPATH rule must refuse the path, not accept it */
+    is_path=0;
     %mp_validatecol(_path,ISPATH,is_path)
     if is_path=0 then _bad=1;
   end;

sas/sasjs/macros/mpe_writecheck.test.sas

@@ -3,8 +3,10 @@
   @brief Testing mpe_writecheck macro
   @details Covers the guard, the three answers, and the probe: it must not
   destroy a file that happens to carry a name it used to use.  The abort paths
-  are covered by services/admin/makedata.test.sas, because mp_abort cannot be
-  exercised inside the test session itself.
+  are not asserted anywhere: mp_abort cannot be exercised inside the test
+  session, and the service that calls this macro refuses the browse actions on
+  any appLoc that carries a settings.sas, which every appLoc running the suite
+  does.
 
   <h4> SAS Macros </h4>
   @li mf_getuniquefileref.sas

sas/sasjs/targets/server/services_server/admin/makedata.sas

@@ -29,6 +29,8 @@
   @li mf_getapploc.sas
   @li mf_mkdir.sas
   @li mp_abort.sas
+  @li mpe_action.sas
+  @li ms_getfile.sas
   @li mpe_dirlist.sas
   @li mpe_writecheck.sas
   @li mpe_makedata.sas
@@ -44,7 +46,7 @@
 
 **/
 
-%global path ADMIN OUTFMT keepme;
+%global path ADMIN OUTFMT keepme action;
 
 /* response format - HTML (default) or JSON */
 %let OUTFMT=HTML;
@@ -56,46 +58,12 @@
 %let inds=%sysfunc(ifc(&exist=1,fromjs,_null_));
 
 /*
-  Optional ACTION column.  This service also answers the deploy screen's
-  questions about the filesystem, so that a deployment has one service to run
-  and the same one removes it.  Only the three known values are ever put into
-  a macro variable - the column comes from the request, so anything else is
-  rejected rather than resolved as macro text.
+  ACTION is required, and arrives either as a column of the input table or as a
+  macro variable (a URL parameter on the platforms that deliver one).  The
+  resolution lives in mpe_action, which reads any incoming value before it
+  assigns one - a default written first is what discarded the parameter.
 */
-/*
-  A URL parameter arrives as a macro variable before this program runs, so its
-  value is captured before anything is assigned - a %let here would overwrite
-  it and the configurator's &action= would be discarded before it was read.
-  symexist guards the read: symget() on a name that is not there writes a
-  note to the log.
-*/
-%let url_action=;
-data _null_;
-  if symexist('action') then call symputx('url_action',symget('action'));
-  stop;
-run;
-%let action=MISSING;
-
-/*
-  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_;
-  length _act $10;
-  _act='';
-  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('url_action')));
-  /* absent is a different refusal from unrecognised */
-  if missing(_act) then _act='MISSING';
-  if _act='MISSING' or _act in ('MAKEDATA','DIRLIST','WRITECHECK')
-    then call symputx('action',_act,'g');
-  else call symputx('action','BAD','g');
-run;
+%mpe_action()
 
 /* both refusals carry their own condition, so neither needs a branch */
 %mp_abort(iftrue= (&action=MISSING)
@@ -305,6 +273,39 @@ run;
 
 %mend makedata_body;
 
+/**
+  * Refuses the browse actions once this appLoc has a settings.sas.
+  *
+  * The deploy window closes when MAKEDATA writes that file, and a run that
+  * failed after writing it leaves the file behind - so its presence, not the
+  * survival of this STP, is what says the system is configured.  Without this
+  * a failed or abandoned install leaves a caller with a directory listing of
+  * any path the SAS user can read.
+  *
+  * A probe that cannot answer is treated as configured: an unreachable drive
+  * is not evidence that the deployment window is still open.
+  */
+%macro browse_gate(iftrue=0);
+%local status;
+%if not(&iftrue) %then %return;
+%let status=0;
+%ms_getfile(%mf_getapploc()/services/public/settings.sas,outref=dcstngs)
+%if %symexist(SYS_PROCHTTP_STATUS_CODE) %then
+  %let status=&SYS_PROCHTTP_STATUS_CODE;
+/* the probe is allowed to 404, so the status is recorded and cleared */
+%let syscc=0;
+%mp_abort(iftrue= (&status=200)
+  ,mac=&_program
+  ,msg=%str(DIRLIST and WRITECHECK are only available until DC is configured)
+)
+%mp_abort(iftrue= (&status ne 200 and &status ne 404)
+  ,mac=&_program
+  ,msg=%str(Could not check whether this appLoc is configured - probe &status)
+)
+%mend browse_gate;
+
+%browse_gate(iftrue= (&action=DIRLIST or &action=WRITECHECK))
+
 /*
   Three invocations rather than one branch: each macro takes `iftrue=` and
   returns immediately when its action is not the one requested, so this file

sas/sasjs/targets/server/services_server/admin/makedata.test.sas

@@ -1,19 +1,19 @@
 /**
   @file
-  @brief testing the makedata service - filesystem browse actions
+  @brief testing the makedata service - action dispatch and the browse gate
   @details
   The deploy screen finds a writeable location before the control library
   exists, so those two questions are answered by the makedata service itself
-  (ACTION=DIRLIST, ACTION=WRITECHECK).  This covers both, and the dispatch
-  that leaves the original behaviour alone when ACTION is absent.
+  (ACTION=DIRLIST, ACTION=WRITECHECK).  The listing and write-probe behaviour
+  is asserted against the macros directly (macros/mpe_dirlist.test.sas,
+  macros/mpe_writecheck.test.sas); this file covers the dispatch around them.
 
-  The fixture is built inside the control library folder, which is a physical
-  path shared with the child service session.
+  The browse actions are only reachable while the appLoc has no settings.sas,
+  and any appLoc running this suite has one - so here the two actions are
+  expected to be refused by the gate.  What that proves is that the action
+  reached the gate at all, which is the part the dispatch owns.
 
   <h4> SAS Macros </h4>
-  @li mf_getuniquefileref.sas
-  @li mf_mkdir.sas
-  @li mp_assert.sas
   @li mpe_assert_abort.sas
   @li mpe_execute.sas
 
@@ -23,230 +23,9 @@
 
 %let _program=&appLoc/services/admin/makedata;
 
-%macro countobs(libds,outmac);
-  %global &outmac;
-  proc sql noprint;
-  select count(*) into:&outmac from &libds
-    where filepath is not null and filepath ne '';
-  quit;
-%mend countobs;
-
-%let proberoot=%sysfunc(pathname(&mpelib))/probe%mf_getuniquefileref();
-%mf_mkdir(&proberoot)
-%mf_mkdir(&proberoot/aa)
-%mf_mkdir(&proberoot/aa/bb)
-%mf_mkdir(&proberoot/aa/bb/cc)
-%mf_mkdir(&proberoot/aa/bb/cc/dd)
-data _null_;
-  file "&proberoot/plain.txt";
-  put 'x';
-run;
-
 /**
-  * Test 1 - one level, folders only
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent="&proberoot";
-  depth=1;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web1,
-  failfast=0
-)
-%countobs(web1.dirlist,c1)
-%mp_assert(
-  iftrue=(&c1=1),
-  desc=Default depth returns the immediate subfolder only,
-  outds=work.test_results
-)
-data _null_;
-  set web1.dirlist;
-  call symputx('name1',filename,'l');
-  call symputx('type1',file_or_folder,'l');
-run;
-%mp_assert(
-  iftrue=("%upcase(&name1)"="AA" and "%upcase(&type1)"="FOLDER"),
-  desc=The file beside it is not offered as a folder,
-  outds=work.test_results
-)
-
-/**
-  * Test 2 - two levels
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent="&proberoot";
-  depth=2;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web2,
-  failfast=0
-)
-%countobs(web2.dirlist,c2)
-%mp_assert(
-  iftrue=(&c2=2),
-  desc=Depth 2 adds the grandchild,
-  outds=work.test_results
-)
-
-/**
-  * Test 3 - four levels asked for, three returned
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent="&proberoot";
-  depth=9;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web3,
-  failfast=0
-)
-%countobs(web3.dirlist,c3)
-%mp_assert(
-  iftrue=(&c3=3),
-  desc=Depth is capped at three levels,
-  outds=work.test_results
-)
-
-/**
-  * Test 4 - the row cap, and the flag that says it was hit
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent="&proberoot";
-  depth=3;
-  maxobs=2;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web4,
-  failfast=0
-)
-%countobs(web4.dirlist,c4)
-data _null_;
-  set web4.dirinfo;
-  call symputx('trunc4',truncated,'l');
-run;
-%mp_assert(
-  iftrue=(&c4=2 and "%upcase(&trunc4)"="Y"),
-  desc=Rows are capped by maxobs and the response says so,
-  outds=work.test_results
-)
-
-/**
-  * Test 5 - a parent that does not exist is an empty listing, not an error
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent="&proberoot/does_not_exist";
-  depth=1;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web5,
-  outstatus=work.st5,
-  failfast=0
-)
-data _null_;
-  set work.st5 end=eof;
-  if eof then call symputx('st5',state,'l');
-run;
-%mp_assert(
-  iftrue=("%upcase(&st5)"="COMPLETED" or "%upcase(&st5)"="UNKNOWN"),
-  desc=A missing parent is not an error,
-  outds=work.test_results
-)
-
-/**
-  * Test 6 - a parent carrying macro triggers is refused before it runs
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent='%nrstr(&proberoot)';
-  depth=1;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web6,
-  failfast=0
-)
-%mpe_assert_abort(
-  desc=A parent with macro triggers is refused,
-  outstatus=work.dc_childstatus,
-  msg=Invalid parent,
-  outds=work.test_results
-)
-
-/**
-  * Test 7 - a relative path is not a location that can be browsed
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent='some/relative/dir';
-  depth=1;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web7,
-  failfast=0
-)
-%mpe_assert_abort(
-  desc=A relative path is refused,
-  outstatus=work.dc_childstatus,
-  msg=Invalid parent,
-  outds=work.test_results
-)
-
-/**
-  * Test 8 - a blank parent is the platform roots, which mp_dirlist answers
-  */
-data work.fromjs;
-  length action $10 parent $1000;
-  action='DIRLIST';
-  parent='';
-  depth=1;
-  maxobs=2000;
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=web8,
-  failfast=0
-)
-%countobs(web8.dirlist,c8)
-%mp_assert(
-  iftrue=(&c8>=1),
-  desc=A blank parent returns the platform roots,
-  outds=work.test_results
-)
-
-/**
-  * Test 9 - an ACTION that is not one of the two, or MAKEDATA, is refused
-    rather than silently seeding the control library
+  * Test 1 - an ACTION that is not one of the three is refused rather than
+    silently seeding the control library
   */
 data work.fromjs;
   length action $10;
@@ -255,7 +34,7 @@ run;
 %mpe_execute(&_program,
   viyacontext=&defaultcontext,
   inputdatasets=work.fromjs,
-  outlib=web9,
+  outlib=web1,
   failfast=0
 )
 %mpe_assert_abort(
@@ -266,107 +45,7 @@ run;
 )
 
 /**
-  * Test 10 - a writeable folder is reported as writeable
-  */
-data work.fromjs;
-  length action $10 path $1000;
-  action='WRITECHECK';
-  path="&proberoot";
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=cw1,
-  failfast=0
-)
-data _null_;
-  set cw1.checkwrite;
-  call symputx('cw1e',path_exists,'l');
-  call symputx('cw1d',is_folder,'l');
-  call symputx('cw1w',writeable,'l');
-run;
-%mp_assert(
-  iftrue=("%upcase(&cw1e)"="Y" and "%upcase(&cw1d)"="Y"
-    and "%upcase(&cw1w)"="Y"),
-  desc=A writable folder is reported as writeable,
-  outds=work.test_results
-)
-
-/**
-  * Test 11 - a file is not a folder, and not writeable as a location
-  */
-data work.fromjs;
-  length action $10 path $1000;
-  action='WRITECHECK';
-  path="&proberoot/plain.txt";
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=cw2,
-  failfast=0
-)
-data _null_;
-  set cw2.checkwrite;
-  call symputx('cw2e',path_exists,'l');
-  call symputx('cw2d',is_folder,'l');
-  call symputx('cw2w',writeable,'l');
-run;
-%mp_assert(
-  iftrue=("%upcase(&cw2e)"="Y" and "%upcase(&cw2d)"="N"
-    and "%upcase(&cw2w)"="N"),
-  desc=A file is not reported as a writeable folder,
-  outds=work.test_results
-)
-
-/**
-  * Test 12 - a path that is not there is a negative answer, not an error
-  */
-data work.fromjs;
-  length action $10 path $1000;
-  action='WRITECHECK';
-  path="&proberoot/nothing_here";
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=cw3,
-  failfast=0
-)
-data _null_;
-  set cw3.checkwrite;
-  call symputx('cw3e',path_exists,'l');
-  call symputx('cw3w',writeable,'l');
-run;
-%mp_assert(
-  iftrue=("%upcase(&cw3e)"="N" and "%upcase(&cw3w)"="N"),
-  desc=A missing path is reported as missing and not writeable,
-  outds=work.test_results
-)
-
-/**
-  * Test 13 - a path carrying macro triggers is refused
-  */
-data work.fromjs;
-  length action $10 path $1000;
-  action='WRITECHECK';
-  path='%nrstr(&proberoot)';
-run;
-%mpe_execute(&_program,
-  viyacontext=&defaultcontext,
-  inputdatasets=work.fromjs,
-  outlib=cw4,
-  failfast=0
-)
-%mpe_assert_abort(
-  desc=A path with macro triggers is refused,
-  outstatus=work.dc_childstatus,
-  msg=Invalid path,
-  outds=work.test_results
-)
-
-/**
-  * Test 15 - ACTION is required: a caller that does not say what it wants is
+  * Test 2 - ACTION is required: a caller that does not say what it wants is
     refused rather than having a control library seeded for it
   */
 data work.fromjs;
@@ -377,7 +56,7 @@ run;
 %mpe_execute(&_program,
   viyacontext=&defaultcontext,
   inputdatasets=work.fromjs,
-  outlib=web10,
+  outlib=web2,
   failfast=0
 )
 %mpe_assert_abort(
@@ -388,12 +67,12 @@ run;
 )
 
 /**
-  * Test 16 - no input table at all: ACTION is not defaulted, so a caller that
+  * Test 3 - 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,
+  outlib=web3,
   failfast=0
 )
 %mpe_assert_abort(
@@ -403,6 +82,50 @@ run;
   outds=work.test_results
 )
 
+/**
+  * Test 4 - DIRLIST is refused once the appLoc is configured.  A run that
+    failed after writing settings.sas leaves it behind, so the file is the
+    test, not the survival of this STP.
+  */
+data work.fromjs;
+  length action $10 parent $1000;
+  action='DIRLIST';
+  parent='/tmp';
+run;
+%mpe_execute(&_program,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.fromjs,
+  outlib=web4,
+  failfast=0
+)
+%mpe_assert_abort(
+  desc=DIRLIST is refused once the appLoc is configured,
+  outstatus=work.dc_childstatus,
+  msg=%str(DIRLIST and WRITECHECK are only available until DC is configured),
+  outds=work.test_results
+)
+
+/**
+  * Test 5 - and so is WRITECHECK
+  */
+data work.fromjs;
+  length action $10 path $1000;
+  action='WRITECHECK';
+  path='/tmp';
+run;
+%mpe_execute(&_program,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.fromjs,
+  outlib=web5,
+  failfast=0
+)
+%mpe_assert_abort(
+  desc=WRITECHECK is refused once the appLoc is configured,
+  outstatus=work.dc_childstatus,
+  msg=%str(DIRLIST and WRITECHECK are only available until DC is configured),
+  outds=work.test_results
+)
+
 /**
   * dump results to the log for offline inspection
   */

sas/sasjs/targets/viya/services_viya/admin/makedata.sas

@@ -23,6 +23,8 @@
   @li mf_mkdir.sas
   @li mf_trimstr.sas
   @li mp_abort.sas
+  @li mpe_action.sas
+  @li mfv_existfile.sas
   @li mpe_dirlist.sas
   @li mpe_writecheck.sas
   @li mpe_makedata.sas
@@ -42,7 +44,7 @@
 **/
 
 
-%global dcpath ADMIN _contextname keepme;
+%global dcpath ADMIN _contextname keepme action;
 
 %webout(FETCH)
 
@@ -58,39 +60,12 @@
   rejected rather than resolved as macro text.
 */
 /*
-  A URL parameter arrives as a macro variable before this program runs, so its
-  value is captured before anything is assigned - a %let here would overwrite
-  it and the configurator's &action= would be discarded before it was read.
-  symexist guards the read: symget() on a name that is not there writes a
-  note to the log.
+  ACTION is required, and arrives either as a column of the input table or as a
+  macro variable (a URL parameter on the platforms that deliver one).  The
+  resolution lives in mpe_action, which reads any incoming value before it
+  assigns one - a default written first is what discarded the parameter.
 */
-%let url_action=;
-data _null_;
-  if symexist('action') then call symputx('url_action',symget('action'));
-  stop;
-run;
-%let action=MISSING;
-
-/*
-  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_;
-  length _act $10;
-  _act='';
-  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('url_action')));
-  /* absent is a different refusal from unrecognised */
-  if missing(_act) then _act='MISSING';
-  if _act='MISSING' or _act in ('MAKEDATA','DIRLIST','WRITECHECK')
-    then call symputx('action',_act,'g');
-  else call symputx('action','BAD','g');
-run;
+%mpe_action()
 
 /* both refusals carry their own condition, so neither needs a branch */
 %mp_abort(iftrue= (&action=MISSING)
@@ -390,6 +365,27 @@ run;
 
 %mend makedata_body;
 
+/**
+  * Refuses the browse actions once this appLoc has a settings.sas.
+  *
+  * The deploy window closes when MAKEDATA writes that file, and a run that
+  * failed after writing it leaves the file behind - so its presence, not the
+  * survival of this STP, is what says the system is configured.  Without this
+  * a failed or abandoned install leaves a caller with a directory listing of
+  * any path the SAS user can read.
+  */
+%macro browse_gate(iftrue=0);
+%local configured;
+%if not(&iftrue) %then %return;
+%let configured=%mfv_existfile(%mf_getapploc()/services/settings.sas);
+%mp_abort(iftrue= (&configured=1)
+  ,mac=&_program
+  ,msg=%str(DIRLIST and WRITECHECK are only available until DC is configured)
+)
+%mend browse_gate;
+
+%browse_gate(iftrue= (&action=DIRLIST or &action=WRITECHECK))
+
 /*
   Three invocations rather than one branch: each macro takes `iftrue=` and
   returns immediately when its action is not the one requested, so this file