fix(deploy): make the folder picker work on both estates

Three defects behind staging's "Invalid parent" and the picker that could
not list a subfolder.  Found by running the real mpe_dirlist on
sas.4gl.io, not by reading it.

1. The browser host answers SYSSCPL "browser".  The client looked for the
   first letter of a unix token (L/H/A/S) and fell through to the Windows
   default for anything else, so the deploy screen pre-filled
   C:\DataController on a POSIX runtime, which the server then refused as
   an invalid parent.  Windows is now recognised by its own tokens (WIN,
   DNTHOST); anything else, including a value the list has never seen,
   gets the POSIX path.

2. mpe_dirlist emitted a non-blank parent's children at LEVEL 0, because
   mp_dirlist numbers a folder's contents with the folder's own level.
   The picker keeps LEVEL>0 for a non-root listing, so every row was
   discarded and a subfolder could never be browsed.  The children are
   shifted to start at 1; a blank parent still lists the platform roots
   at 0.  Verified on the estate: /tmp at DEPTH=1 returns levels [1],
   a blank parent returns [0, 1] (the root, then its children).

3. The mock's write probe generated one name and wrote it, truncating a
   file that was already called that; mpe_writecheck tries up to ten
   names with fileexist()=0 first.  The mock now does the same and
   reports 'No free name for the write probe' when all ten are taken.

The mock's DIRLIST now matches the estate's shape too: children at
LEVEL 1, roots at 0 followed by their children, and the cap stops the
walk at MAXOBS rather than slicing afterwards.  Tests: 35 pass across
the five mock suites (makedata-actions 15, two of them new: the probe
collision and a folder that cannot be written to), sasjs lint clean,
client spec 8/8 with the new platform cases and 2 failing without the
fix.

client/src/app/deploy/sections/sasjs-configurator/sasjs-configurator.component.spec.ts

@@ -87,3 +87,57 @@ describe('SasjsConfiguratorComponent (SAS9 / SASjs setup)', () => {
     expect(spy).toHaveBeenCalled()
   })
 })
+
+/**
+ * The deploy screen pre-fills the directory from SYSSCPL.  A token it does not
+ * recognise used to fall through to the Windows default, which offers the
+ * server a drive path it refuses - so an unknown value has to mean POSIX.
+ */
+describe('SasjsConfiguratorComponent directory default', () => {
+  const directoryFor = async (sysscpl: any) => {
+    const sasService: any = {
+      request: () =>
+        Promise.resolve({
+          adapterResponse: {
+            MF_GETUSER: 'demo',
+            SYSUSERID: '0',
+            SYSHOSTNAME: 'host',
+            SYSVLONG: 'browser',
+            SYSSCPL: sysscpl,
+            groups: [{ GROUPNAME: 'AllUsers' }]
+          }
+        })
+    }
+    const sasjsService: any = {
+      getServerInfo: () => ({ subscribe: () => undefined })
+    }
+    const component = new SasjsConfiguratorComponent(sasService, sasjsService, {
+      replaceState: () => undefined
+    } as any)
+
+    component.getUserGroups()
+    // the default is set in the promise callback this call schedules
+    await Promise.resolve()
+    await Promise.resolve()
+
+    return component.dcDirectory
+  }
+
+  it('gives the POSIX path for the browser host, whose token is not a platform', async () => {
+    expect(await directoryFor('browser')).toBe('/tmp/DataController')
+  })
+
+  it('gives the POSIX path for the unix tokens', async () => {
+    expect(await directoryFor('LINUX')).toBe('/tmp/DataController')
+    expect(await directoryFor('AIX')).toBe('/tmp/DataController')
+  })
+
+  it('gives the drive path for the Windows tokens', async () => {
+    expect(await directoryFor('WIN')).toBe('C:\\DataController')
+    expect(await directoryFor('DNTHOST')).toBe('C:\\DataController')
+  })
+
+  it('gives the POSIX path when the token is absent', async () => {
+    expect(await directoryFor(undefined)).toBe('/tmp/DataController')
+  })
+})

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

@@ -89,20 +89,24 @@ export class SasjsConfiguratorComponent implements OnInit {
           user who is installing Data Controller.  This path should be
           appropriate to the server type, ie windows or linux.
           We can use the SYSSCPL variable from the service response above, and
-          derive the OS based on the first letter.
+          derive the OS from it.
 
           The list of values is available here:
 
           https://documentation.sas.com/doc/en/pgmsascdc/9.4_3.5/mcrolref/n0e7s8lf147kazn17u45trpwb6sw.htm
 
+          Windows is recognised by its own tokens (WIN, DNTHOST).  Anything
+          else gets the POSIX path: the previous test looked for the first
+          letter of a unix token, so a value it did not know - the
+          browser-hosted build answers "browser" - fell through to a drive
+          path, and the server then refused the folder it had been offered.
         */
+        const sysscpl = String(res.adapterResponse.SYSSCPL || '').toUpperCase()
         this.dcDirectory =
           this.tmpDirectories[
-            ['L', 'H', 'A', 'S'].includes(
-              res.adapterResponse.SYSSCPL.substring(0, 1)
-            )
-              ? 'linux'
-              : 'windows'
+            sysscpl.startsWith('W') || sysscpl.startsWith('D')
+              ? 'windows'
+              : 'linux'
           ]
 
         this.dcAdminGroupList = res.adapterResponse.groups

sas/mocks/sasjs/services/admin/makedata.js

@@ -160,54 +160,67 @@ function brDirList(parent, depth, maxobs) {
   const rows = []
   /* the cap bounds the walk, not only the response: browsing /usr or /proc at
     depth 3 must not stat everything below it and then throw most of it away */
-  const full = () => rows.length > maxobs
+  let capped = false
+  const full = () => {
+    if (rows.length < maxobs) return false
+    capped = true
+    return true
+  }
 
+  /*
+    LEVEL is the level the entries sit at, and the picker keeps LEVEL>0 for a
+    non-root listing - so children start at 1.  mp_dirlist is the reference: on
+    the estate a DEPTH=1 browse of /tmp comes back with its folders at level 1,
+    and a blank parent returns the platform roots at level 0 followed by their
+    children at level 1.
+  */
+  let frontier = []
   if (!parent) {
     for (const root of brRoots()) {
       if (full()) break
       rows.push(brFolderRow(root, 0, 'folder', ''))
+      frontier.push(root)
     }
   } else {
-    rows.push(brFolderRow(parent, 0, 'folder', ''))
-    let frontier = [parent]
-    walk: for (let level = 1; level <= depth; level++) {
-      const next = []
-      for (const folder of frontier) {
-        if (full()) break walk
-        let names
+    frontier = [parent]
+  }
+
+  for (let level = 1; level <= depth; level++) {
+    const next = []
+    for (const folder of frontier) {
+      if (full()) break
+      let names
+      try {
+        names = fs.readdirSync(folder).slice().sort()
+      } catch (err) {
+        continue
+      }
+      for (const name of names) {
+        if (full()) break
+        const child = nodePath.join(folder, name)
+        let isFolder = false
         try {
-          names = fs.readdirSync(folder).slice().sort()
+          isFolder = fs.statSync(child).isDirectory()
         } catch (err) {
+          rows.push(brFolderRow(child, level, 'locked', err.message))
           continue
         }
-        for (const name of names) {
-          if (full()) break walk
-          const child = nodePath.join(folder, name)
-          let isFolder = false
-          try {
-            isFolder = fs.statSync(child).isDirectory()
-          } catch (err) {
-            rows.push(brFolderRow(child, level, 'locked', err.message))
-            continue
-          }
-          if (!isFolder) continue
-          rows.push(brFolderRow(child, level, 'folder', ''))
-          next.push(child)
-        }
+        if (!isFolder) continue
+        rows.push(brFolderRow(child, level, 'folder', ''))
+        next.push(child)
       }
-      frontier = next
-      if (!frontier.length) break
     }
+    frontier = next
+    if (!frontier.length) break
   }
 
   rows.sort(function (a, b) {
     return a.FILEPATH < b.FILEPATH ? -1 : a.FILEPATH > b.FILEPATH ? 1 : 0
   })
 
-  const truncated = rows.length > maxobs
   return {
-    rows: truncated ? rows.slice(0, maxobs) : rows,
-    truncated: truncated
+    rows: rows.slice(0, maxobs),
+    truncated: capped
   }
 }
 
@@ -248,13 +261,27 @@ function brWriteCheck(path) {
   }
   row.IS_FOLDER = 'Y'
 
-  const probe = nodePath.join(
-    target,
-    '.dcprobe_' +
-      (typeof process !== 'undefined' && process.pid ? process.pid : 0) +
-      '_' +
-      Math.floor(Math.random() * 1e9)
-  )
+  /* mpe_writecheck tries up to ten names with fileexist()=0 before fopen('O'),
+    so a probe never truncates a file that is already there */
+  let probe = ''
+  for (let attempt = 1; attempt <= 10; attempt++) {
+    const candidate = nodePath.join(
+      target,
+      '.dcprobe_' +
+        (typeof process !== 'undefined' && process.pid ? process.pid : 0) +
+        '_' +
+        Math.floor(Math.random() * 1e9)
+    )
+    if (!fs.existsSync(candidate)) {
+      probe = candidate
+      break
+    }
+  }
+  if (!probe) {
+    row.MSG = 'No free name for the write probe'
+    return row
+  }
+
   try {
     fs.writeFileSync(probe, '')
     fs.unlinkSync(probe)

sas/mocks/test/makedata-actions.test.js

@@ -44,10 +44,16 @@ test('a browse does not seed the control library', () => {
   const body = parse(webout)
   assert.equal(body.dirinfo[0].PARENT, '')
   assert.equal(body.dirinfo[0].TRUNCATED, 'N')
+  // a blank parent is the platform roots at LEVEL 0 (mp_dirlist also expands
+  // them one level, which is what the SAS service returns for DEPTH=1)
   assert.deepEqual(
-    body.dirlist.map((row) => row.FILEPATH),
+    body.dirlist.filter((row) => row.LEVEL === 0).map((row) => row.FILEPATH),
     ['/'],
-    'a blank parent lists the platform roots'
+    'the platform root is listed at LEVEL 0'
+  )
+  assert.ok(
+    body.dirlist.every((row) => row.LEVEL === 0 || row.LEVEL === 1),
+    'children of the root come back at LEVEL 1'
   )
 
   // the failure this test exists for: nothing written, program still there
@@ -89,11 +95,10 @@ test('DIRLIST lists the folders below the parent, one level deep', () => {
   assert.deepEqual(
     body.dirlist.map((row) => [row.FILENAME, row.LEVEL, row.FILE_OR_FOLDER]),
     [
-      [path.basename(estate.pickme), 0, 'folder'],
       ['child', 1, 'folder'],
       ['second', 1, 'folder']
     ],
-    'the parent at level 0, then its folders - a file is not offered'
+    'the folders at LEVEL 1, the level the picker keeps - a file is not offered'
   )
 })
 
@@ -107,13 +112,14 @@ test('DIRLIST caps the listing and says it was truncated', () => {
           ACTION: 'DIRLIST',
           PARENT: estate.pickme,
           DEPTH: 1,
-          MAXOBS: 2
+          MAXOBS: 1
         }
       ]
     })
   )
 
-  assert.equal(body.dirlist.length, 2)
+  // one folder fits, the second is refused rather than walked and discarded
+  assert.equal(body.dirlist.length, 1)
   assert.equal(body.dirinfo[0].TRUNCATED, 'Y')
 })
 
@@ -207,6 +213,63 @@ test('WRITECHECK reports a file as not a folder', () => {
   assert.equal(row.MSG, 'Path exists but is not a folder')
 })
 
+test('WRITECHECK reports a folder it cannot write to', (t) => {
+  const estate = makeEstate()
+  const dir = estate.readonly
+  fs.chmodSync(dir, 0o555)
+
+  if (typeof process.getuid === 'function' && process.getuid() === 0) {
+    fs.chmodSync(dir, 0o755)
+    t.skip('running as root, where a read-only folder is still writable')
+    return
+  }
+
+  try {
+    const row = parse(
+      runMakedata(estate, { rows: [{ ACTION: 'WRITECHECK', PATH: dir }] })
+    ).checkwrite[0]
+
+    assert.equal(row.PATH_EXISTS, 'Y')
+    assert.equal(row.IS_FOLDER, 'Y')
+    assert.equal(row.WRITEABLE, 'N')
+    assert.ok(row.MSG, 'the runtime message is passed on')
+  } finally {
+    fs.chmodSync(dir, 0o755)
+  }
+})
+
+test('the write probe never overwrites a file that is already there', () => {
+  const estate = makeEstate()
+
+  // pin the random part so the probe name is predictable, then occupy it
+  const realRandom = Math.random
+  Math.random = () => 0.5
+  const probe = path.join(
+    estate.pickme,
+    '.dcprobe_' + process.pid + '_' + Math.floor(0.5 * 1e9)
+  )
+  fs.writeFileSync(probe, 'someone else\'s file')
+
+  try {
+    const row = parse(
+      runMakedata(estate, {
+        rows: [{ ACTION: 'WRITECHECK', PATH: estate.pickme }]
+      })
+    ).checkwrite[0]
+
+    // mpe_writecheck gives up after ten names rather than truncating one
+    assert.equal(row.WRITEABLE, 'N')
+    assert.equal(row.MSG, 'No free name for the write probe')
+    assert.equal(
+      fs.readFileSync(probe, 'utf8'),
+      "someone else's file",
+      'the existing file is untouched'
+    )
+  } finally {
+    Math.random = realRandom
+  }
+})
+
 test('WRITECHECK refuses a missing or relative path', () => {
   const estate = makeEstate()
 

sas/sasjs/macros/mpe_dirlist.sas

@@ -134,6 +134,21 @@ data work.dirlist;
   end;
 run;
 
+/*
+  mp_dirlist numbers a folder's children with the folder's own level, so the
+  contents of a non-blank PARENT arrive at level 0 - the level the picker
+  reserves for the parent row it does not draw (it keeps LEVEL>0 for a non-root
+  listing).  Shift the children to start at 1, or the picker is handed rows it
+  discards and a subfolder can never be browsed.  A blank parent lists the
+  platform roots, which are the top level and stay at 0.
+*/
+%if %length(%superq(br_parent)) > 0 %then %do;
+  data work.dirlist;
+    set work.dirlist;
+    level=level+1;
+  run;
+%end;
+
 /* a predictable order makes the picker stable between calls */
 proc sort data=work.dirlist;
   by filepath;

sas/sasjs/macros/mpe_dirlist.test.sas

@@ -85,6 +85,15 @@ run;
   desc=The file beside it is not offered,
   outds=work.test_results
 )
+data _null_;
+  set work.dirlist;
+  call symputx('level2',level,'l');
+run;
+%mp_assert(
+  iftrue=(&level2=1),
+  desc=Children of the parent are level 1, the level the picker keeps,
+  outds=work.test_results
+)
 
 /**
   * Test 3 - depth and its cap